feat: add --git-client to explicitly set the type of forge #106
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
Coverage report
Test suite run success169 tests passing in 16 suites. Report generated by 🧪jest coverage report action from 5f12c50 |
|
This is probably not how you would like that implemented and I'm ready to make improvements. The original author of the patch may also be willing to improve upon it. |
earl-warren
force-pushed
the
wip-git-client
branch
from
620c485
to
53fe5d1
Compare
|
added tests |
earl-warren
force-pushed
the
wip-git-client
branch
from
a45c3ac
to
0f0812a
Compare
|
Hey @earl-warren , this looks a great idea! I see you already added some tests, thx a lot! After a quick check looks good to me, I will deeply check it in a while 🚀 |
lampajr
requested changes
Mar 23, 2024
There was a problem hiding this comment.
Overall looks great to me!
Just a couple of missing things:
- Could you please add the same
git-clientparam in theaction.yamlfor the GHA - If I run
npm run buildlocally I see the following diff:
"query",
"search",
- "hash",
];
// Create handlers that pass events from native requests
@@ -6325,7 +6324,7 @@ RedirectableRequest.prototype._processResponse = function (response) {
redirectUrl.protocol !== "https:" ||
redirectUrl.host !== currentHost &&
!isSubdomain(redirectUrl.host, currentHost)) {
- removeMatchingHeaders(/^(?:(?:proxy-)?authorization|cookie)$/i, this._options.headers);
+ removeMatchingHeaders(/^(?:authorization|cookie)$/i, this._options.headers);
}
// Evaluate the beforeRedirect callback
diff --git a/dist/gha/index.js b/dist/gha/index.js
index b50c45c..21cc4e7 100755
--- a/dist/gha/index.js
+++ b/dist/gha/index.js
@@ -7621,7 +7621,6 @@ var preservedUrlFields = [
"protocol",
"query",
"search",
- "hash",
];
// Create handlers that pass events from native requests
@@ -8055,7 +8054,7 @@ RedirectableRequest.prototype._processResponse = function (response) {
redirectUrl.protocol !== "https:" ||
redirectUrl.host !== currentHost &&
!isSubdomain(redirectUrl.host, currentHost)) {
- removeMatchingHeaders(/^(?:(?:proxy-)?authorization|cookie)$/i, this._options.headers);
+ removeMatchingHeaders(/^(?:authorization|cookie)$/i, this._options.headers);
}Do you mind to double check this?
I have no clue why it does that in my environment. I'll remove these hunks and figure that out later. |
earl-warren
force-pushed
the
wip-git-client
branch
from
0f0812a
to
391771b
Compare
|
codeberg is running Forgejo and it may not be possible to infer that from the URL alone. The same is true for GitHub Enterprise Server.
earl-warren
force-pushed
the
wip-git-client
branch
from
391771b
to
5f12c50
Compare
|
Here is the output of earl-warren:~/software/git-backporting$ npm run build
> @kie/git-backporting@4.5.2 build
> npm run clean && npm run compile && npm run package
> @kie/git-backporting@4.5.2 clean
> rm -rf ./build ./dist
> @kie/git-backporting@4.5.2 compile
> tsc -p tsconfig.json && tsc-alias -p tsconfig.json
> @kie/git-backporting@4.5.2 package
> npm run package:cli && npm run package:gha
> @kie/git-backporting@4.5.2 package:cli
> ncc build ./build/src/bin/cli.js -o dist/cli
ncc: Version 0.36.0
ncc: Compiling file index.js into CJS
1105kB dist/cli/index.js
1105kB [950ms] - ncc 0.36.0
> @kie/git-backporting@4.5.2 package:gha
> ncc build ./build/src/bin/gha.js -o dist/gha
ncc: Version 0.36.0
ncc: Compiling file index.js into CJS
1095kB dist/gha/index.js
1095kB [1009ms] - ncc 0.36.0
earl-warren:~/software/git-backporting$ git diff
diff --git a/dist/cli/index.js b/dist/cli/index.js
index a3f1819..77ad9bf 100755
--- a/dist/cli/index.js
+++ b/dist/cli/index.js
@@ -5891,6 +5891,7 @@ var preservedUrlFields = [
"protocol",
"query",
"search",
+ "hash",
];
// Create handlers that pass events from native requests
@@ -6324,7 +6325,7 @@ RedirectableRequest.prototype._processResponse = function (response) {
redirectUrl.protocol !== "https:" ||
redirectUrl.host !== currentHost &&
!isSubdomain(redirectUrl.host, currentHost)) {
- removeMatchingHeaders(/^(?:authorization|cookie)$/i, this._options.headers);
+ removeMatchingHeaders(/^(?:(?:proxy-)?authorization|cookie)$/i, this._options.headers);
}
// Evaluate the beforeRedirect callback
diff --git a/dist/gha/index.js b/dist/gha/index.js
index 21cc4e7..b50c45c 100755
--- a/dist/gha/index.js
+++ b/dist/gha/index.js
@@ -7621,6 +7621,7 @@ var preservedUrlFields = [
"protocol",
"query",
"search",
+ "hash",
];
// Create handlers that pass events from native requests
@@ -8054,7 +8055,7 @@ RedirectableRequest.prototype._processResponse = function (response) {
redirectUrl.protocol !== "https:" ||
redirectUrl.host !== currentHost &&
!isSubdomain(redirectUrl.host, currentHost)) {
- removeMatchingHeaders(/^(?:authorization|cookie)$/i, this._options.headers);
+ removeMatchingHeaders(/^(?:(?:proxy-)?authorization|cookie)$/i, this._options.headers);
}
// Evaluate the beforeRedirect callback |
lampajr
approved these changes
Mar 23, 2024
There was a problem hiding this comment.
Sounds great!
Here is the output of npm run build on my environment. There evidently is a difference in my environment that matters but I don't know how to figure that out. Do you have a suggestion? Not that it matters in the context of this PRs but it would be nice to have clean content the next time around 🙏
To be honest I've never faced this weird issue, do you mind to create an issue adding this diff and some additional infos like the system you are using alongside the npm and node version? Then I can try to investigate a bit more if I can find something / or to reproduce.
Anyway as you said this is not blocking this PR and it looks great to me!
Thanks a lot for your contribution @earl-warren ❤️
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.
codeberg is running Forgejo and it may not be possible to infer that from the URL alone. The same is true for GitHub Enterprise Server.