Skip to content
This repository was archived by the owner on Aug 6, 2025. It is now read-only.

Conversation

@rayangler
Copy link
Contributor

@rayangler rayangler commented Mar 21, 2025

Stories/Links:

DOP-5440

Related PRs:

Notes

  • This will ignore github_username when upserting build artifacts through the persistence module.
  • This PR should be merged after the updated_documents collection no longer has duplicated page_ids.

README updates

    • This PR introduces changes that should be reflected in the README, and I have made those updates.
    • This PR does not introduce changes that should be reflected in the README

@github-actions
Copy link

Your feature branch infrastructure has been deployed!

Your webhook URL is: https://8ext2l4ca9.execute-api.us-east-2.amazonaws.com/prod/webhook/githubEndpoint/trigger/build

For more information on how to use this endpoint, follow these instructions.

Copy link
Contributor

@mmeigs mmeigs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple nits. Also, I thought the persistence module was now solely in netlify extensions? I didn't know this code was still in use...

Comment on lines 38 to 39
// Safely convert jobId in case of empty string
const autobuilderJobId = jobId || undefined;
const autobuilderJobId = jobId;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we remove the comment now? Or should we keep the fallback to keep the comment true?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! I ended up just removing autobuilderJobId altogether since it's redundant with jobId being inherently of string | undefined type anyways


describe('metadataFromZip', () => {
it('should get metadata from site.bson', async () => {
const githubUser = 'gritty';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol i'll miss it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we can remove GH_USER constant

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! Done

@rayangler
Copy link
Contributor Author

I thought the persistence module was now solely in netlify extensions? I didn't know this code was still in use...

Yup, the extensions download the code from this repo and use it. We can probably move the persistence module logic directly into the extensions in the future

@rayangler rayangler requested a review from mmeigs March 24, 2025 14:15
@rayangler rayangler merged commit 1070c13 into main Mar 24, 2025
9 checks passed
@rayangler rayangler deleted the DOP-5440-github-user branch March 24, 2025 20:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants