Do not cache the output of gitAskPass
#3250
Merged
Conversation
This file contains 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
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3250 +/- ##
==========================================
+ Coverage 91.17% 91.18% +0.01%
==========================================
Files 166 167 +1
Lines 3400 3404 +4
Branches 310 306 -4
==========================================
+ Hits 3100 3104 +4
Misses 300 300 ☔ View full report in Codecov by Sentry. |
Prior to this change, the `gitAskPass` program was called once on start-up and its output was cached and used for API calls to the forge while Git itself does not cache its output but calls it anytime a password is needed. This means if the output of `gitAskPass` changes during a Scala Steward run, the new password is only used for Git operations but not for forge API calls. With this change we now do the same as Git and call `gitAskPass` everytime the password is needed. This should make it easier to support GitHub Apps proper which require different access tokens during a run. See also #2973 (comment).
fthomas
force-pushed
the
topic/do-not-cache-credentials
branch
from
c512a04 to
ff2098a
Compare
I tried this successfully in https://github.com/scala-steward-org/scala-steward/compare/topic/gh-app-2 which modifies |
fthomas
added a commit
that referenced
this pull request
Dec 23, 2023
This removes the unnecessary `Repo` parameter from the function for modifying requests to add authentication headers. This parameter was added in #373 but wasn't ever used as can be observed in the diff of `ForgeSelection`. My motivation for adding `Repo` back then was to eventually use different credentials for different repositories for proper GitHub App support. But #3250 demonstrates that we can achieve this without this extra parameter.
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.
Prior to this change, the
gitAskPassprogram was called once on start-up and its output was cached and used for API calls to the forge while Git itself does not cache its output but calls it anytime a password is needed. This means if the output ofgitAskPasschanges during a Scala Steward run, the new password is only used for Git operations but not for forge API calls.With this change we now do the same as Git and call
gitAskPasseverytime the password is needed. This should make it easier to support GitHub Apps proper which require different access tokens during a run. See also #2973 (comment).