-
Notifications
You must be signed in to change notification settings - Fork 2
fix: update geolite workflow #40
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the GeoLite database workflow to target the release-ulmo branch instead of master and changes the reviewer assignment from an individual to a team.
- Updated default and base branch references from "master" to "release-ulmo"
- Changed PR reviewer from individual (@feanil) to team (edx/orbi-bom)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
.github/workflows/update-geolite-database.yml:27
- The checkout step doesn't specify which branch to check out. Since the workflow accepts a
branchinput parameter, it should check out that branch:ref: '${{ inputs.branch || 'release-ulmo' }}'. Without this, the workflow will check out the default repository branch rather than the intended target branch, potentially causing the PR to be based on incorrect code.
- name: Checkout Repository
uses: actions/checkout@v5
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
bf2c271 to
f872eeb
Compare
f872eeb to
0842fac
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
chintanjoshi-apphelix-2u
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left minor comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| branch: | ||
| description: "Target branch against which to create PR" | ||
| required: false | ||
| default: "master" | ||
| default: "release-ulmo" |
Copilot
AI
Dec 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The branch input parameter is defined but never used in the workflow. The checkout step doesn't use inputs.branch to determine which branch to check out, and the PR base is now hardcoded to 'release-ulmo'.
Consider either:
- Removing the unused
branchinput parameter if it's no longer needed, or - Using it to dynamically set the checkout ref and PR base branch like:
ref: "${{ inputs.branch || 'release-ulmo' }}"in the checkout step and--base '${{ inputs.branch || 'release-ulmo' }}'in the PR creation command.
chintanjoshi-apphelix-2u
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to go
* fix: update geolite workflow
Description
Update update-geolite workflow from master => release-ulmo, and feanil => orbi-bom.
Each named release will need to make an update. This should go on our Verawood doc.
We changed the branch name logic because the workflow regenerated the same branch name on reruns (since it was based on github.sha), causing GitHub to reject the push with “fetch first” because the branch already existed on the remote.
Ticket Link
https://2u-internal.atlassian.net/browse/BOMS-267
Related PR
openedx#37706