Get GH token using expected variable - #323
Draft
mjgiarlo wants to merge 1 commit into
Draft
Conversation
mjgiarlo
commented
May 11, 2026
| # "REPOS_PATH=infrastructure GH_ACCESS_TOKEN=abc123 COCINA_LEVEL2= ./merge-all.rb" | ||
| BRANCH_NAME = ENV.fetch('BRANCH_NAME', 'update-dependencies') | ||
| COCINA_LEVEL2_BRANCH_NAME = 'cocina-level2-updates' | ||
| # To generate update-dependencies PRs for all repos in <REPOS_PATH>/projects.yml: |
Member
Author
There was a problem hiding this comment.
I've never felt that having quotes around the command hints was useful because it makes copy/paste a tad more difficult. Remove the quotes. (I could also be convinced to remove the dollar signs.)
Improve clarity of code comments.
mjgiarlo
commented
May 11, 2026
Comment on lines
+14
to
+16
| # NOTE: The above variables may also be set as environment variables instead, in | ||
| # which case they will be picked up by the script without needing to be | ||
| # passed in on the command line. |
Member
Author
There was a problem hiding this comment.
Clarify that env vars may either be set in the shell session or provided on the command-line.
mjgiarlo
commented
May 11, 2026
Comment on lines
+18
to
20
| require 'highline/import' | ||
| require 'octokit' | ||
| require 'yaml' |
Member
Author
There was a problem hiding this comment.
Co-locate all require statements.
There was a problem hiding this comment.
Pull request overview
Updates the merge-all.rb script to use a more standard GitHub token environment variable and refactors parts of the script’s repo loading and PR status handling.
Changes:
- Updates usage instructions and token lookup to prefer
GITHUB_TOKEN(with fallback toGH_ACCESS_TOKEN). - Refactors projects loading / filtering logic and PR enumeration (uses
filter_map). - Adjusts PR status handling and exit codes for “no PRs found” / “checks failing” cases.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+6
to
+10
| # To generate update-dependencies PRs for all repos in <REPOS_PATH>/projects.yml: | ||
| # | ||
| # $ REPOS_PATH=infrastructure GITHUB_TOKEN=abc123 ./merge-all.rb | ||
| # | ||
| # To generate PRs only for projects that rely on Cocina updates: |
|
|
||
| def access_token | ||
| @access_token = ENV['GITHUB_TOKEN'] || ENV['GH_ACCESS_TOKEN'] | ||
| raise 'GITHUB_TOKEN variable must be set' if @access_token.nil? || @access_token.empty? |
|
|
||
| require 'octokit' | ||
| client = Octokit::Client.new(access_token: access_token) | ||
| client = Octokit::Client.new(access_token:) |
Comment on lines
80
to
+83
| def status_from(statuses, checks) | ||
| # GitHub API marks PRs with 0 statuses as "pending", we cast that to success | ||
| return 'success' if (statuses.state == 'success' || statuses.total_count.zero?) && | ||
| checks.check_runs.map(&:conclusion).all? { |status| status == 'success' } | ||
|
|
||
| 'failure' | ||
| end | ||
|
|
||
| def access_token | ||
| ENV['GH_ACCESS_TOKEN'] | ||
| end | ||
|
|
||
| # anything other than nil or false is true here, just as Ruby intended | ||
| def cocina_level2 | ||
| ENV['COCINA_LEVEL2'] | ||
| (statuses.state == 'success' || statuses.total_count.zero?) && | ||
| checks.check_runs.map(&:conclusion).all? { |status| status == 'success' } |
Comment on lines
+94
to
98
| unless pr_list.all? { |pr| pr[:status] } | ||
| puts '*No* PRs were merged because these PRs are not passing: ' | ||
| pr_list.filter { |pr| pr[:status] != 'success' }.each do |pr| | ||
| pr_list.reject { |pr| pr[:status] }.each do |pr| | ||
| puts "#{pr[:status]} - #{pr[:url]}" | ||
| end |
mjgiarlo
commented
May 11, 2026
| COCINA_LEVEL2_BRANCH_NAME = ENV.fetch('COCINA_LEVEL2_BRANCH_NAME', 'cocina-level2-updates') | ||
|
|
||
| def access_token | ||
| @access_token = ENV['GITHUB_TOKEN'] || ENV['GH_ACCESS_TOKEN'] |
Member
Author
There was a problem hiding this comment.
AFAICT, GITHUB_TOKEN is the recommended and most often used env var for this token, so allow that one to work, but don't break anyone who's been relying on the bespoke (and/or outdated) env var this codebase has been using.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
If this is only a minor change to the scripts, please 🔪 kill the Jenkins build. 🔪
Navigate from SUL CI ➡️ Stanford University Digital Library ➡️ access-update-scripts ➡️ Branches / main ➡️ Build History ➡️ Cancel build button (🆇)