Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIX] Replaced the latest version tag with a baked-in version #390

Merged
merged 74 commits into from
Dec 10, 2024
Merged

Conversation

rmanaem
Copy link
Contributor

@rmanaem rmanaem commented Dec 4, 2024

Changes proposed in this pull request:

  • The version is now read from the package.json
  • Docker workflows have been update:
    • build docker on release:
      • update the version in package.json to match that of GitHub
      • commits the change of version in package.json
      • trigger the deploy workflow
    • build docker nightly:
      • update the version in package.json to the latest SHA
      • don't commit the change in version

Checklist

This section is for the PR reviewer

  • PR has an interpretable title with a prefix ([ENH], [FIX], [REF], [TST], [CI], [MNT], [INF], [MODEL], [DOC]) (see our Contributing Guidelines for more info)
  • PR has a label for the release changelog or skip-release (to be applied by maintainers only)
  • PR links to GitHub issue with mention Closes #XXXX
  • Tests pass
  • Checks pass
  • If the PR changes the participant-level and/or the dataset-level result file, the query-tool-results files in the neurobagel_examples repo have been regenerated

For new features:

  • Tests have been added

For bug fixes:

  • There is at least one test that would fail under the original bug conditions.

Summary by Sourcery

Replace the dynamic fetching of the latest release tag with a static version from package.json and update Docker workflows to manage versioning during builds.

Bug Fixes:

  • Replace the dynamic fetching of the latest release tag with a static version read from package.json in the Navbar component.

CI:

  • Update Docker workflows to handle versioning by reading from package.json and committing changes during release builds.

Copy link

sourcery-ai bot commented Dec 4, 2024

Reviewer's Guide by Sourcery

This PR implements a more reliable versioning system by reading the version from package.json instead of fetching it from GitHub's API. The Docker workflows have been updated to manage this version: the release workflow now updates package.json with the release version and commits the change, while the nightly build workflow updates it with the latest commit SHA without committing.

No diagrams generated as the changes look simple and do not need a visual representation.

File-Level Changes

Change Details Files
Changed version management from GitHub API to package.json
  • Removed GitHub API call to fetch latest release tag
  • Added import of version from package.json
  • Updated version badge to use package.json version
src/components/Navbar.tsx
Enhanced release Docker workflow with version management
  • Added step to extract release version from GitHub event
  • Added GitHub App token generation for authentication
  • Added step to update package.json version
  • Added commit and push of updated package.json
  • Added trigger for deploy workflow
.github/workflows/build_docker_on_release.yml
Updated nightly Docker build workflow
  • Added step to fetch latest commit SHA
  • Added step to update package.json version with SHA
  • Version is updated but not committed
.github/workflows/build_docker_nightly.yml
Modified deploy workflow triggers
  • Removed direct trigger on release publish
  • Added repository dispatch trigger
.github/workflows/deploy.yaml

Assessment against linked issues

Issue Objective Addressed Explanation
#291 Replace the latest version tag fetched from GitHub API with a baked-in version from package.json
#291 Implement correct version handling for release builds (using release version) and nightly builds (using commit SHA)
#291 Ensure version information is baked into Docker image during build process

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time. You can also use
    this command to specify where the summary should be inserted.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

netlify bot commented Dec 4, 2024

Deploy Preview for neurobagel-query ready!

Name Link
🔨 Latest commit 4ab3f86
🔍 Latest deploy log https://app.netlify.com/sites/neurobagel-query/deploys/6758098bba922e0008c6b0ec
😎 Deploy Preview https://deploy-preview-390--neurobagel-query.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @rmanaem - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟡 General issues: 2 issues found
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@rmanaem rmanaem added pr-bug-fix Bug fix, will increment patch version when merged (0.0.+1) release Create a release when this PR is merged labels Dec 4, 2024
@rmanaem rmanaem requested review from surchs and alyssadai December 4, 2024 17:23
Copy link
Contributor

@surchs surchs left a comment

Choose a reason for hiding this comment

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

Thanks @rmanaem for this! Looks very nice and solves an important problem 🎉

I left some comments, mainly clarity. But please also open an issue to explore ways to make the package.json version bump part of the release in the future. I think that'd be clearer

🧑‍🍳

@rmanaem
Copy link
Contributor Author

rmanaem commented Dec 5, 2024

After an in-length discussion, we decided to move to implementing a workflow to run on PR when labels have been changed (action type: labeled, see docs). This workflow will calculate the new version using auto version (see docs), update the package.json.
The application of release label should only happen after the requested changes and comments have been addressed and the PR has been approved to avoid out of sync local and remote.

@rmanaem rmanaem added pr-bug-fix Bug fix, will increment patch version when merged (0.0.+1) release Create a release when this PR is merged and removed pr-bug-fix Bug fix, will increment patch version when merged (0.0.+1) release Create a release when this PR is merged labels Dec 5, 2024
@rmanaem rmanaem added release Create a release when this PR is merged and removed release Create a release when this PR is merged labels Dec 5, 2024
@rmanaem rmanaem removed the release Create a release when this PR is merged label Dec 7, 2024
@rmanaem rmanaem added the release Create a release when this PR is merged label Dec 7, 2024
@rmanaem rmanaem added release Create a release when this PR is merged and removed release Create a release when this PR is merged labels Dec 7, 2024
@rmanaem rmanaem requested review from surchs and alyssadai and removed request for alyssadai December 7, 2024 20:40
Copy link
Contributor

@surchs surchs left a comment

Choose a reason for hiding this comment

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

Thanks @rmanaem, this looks very good!

Left one question, otherwise 🧑‍🍳

@rmanaem rmanaem merged commit f3c5210 into main Dec 10, 2024
9 checks passed
@rmanaem rmanaem deleted the bug-291 branch December 10, 2024 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-bug-fix Bug fix, will increment patch version when merged (0.0.+1) release Create a release when this PR is merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace the latest version tag with a baked-in version
2 participants