-
Notifications
You must be signed in to change notification settings - Fork 199
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
Trim commit message to not include description #226
Conversation
This PR is idle because it has been open for 14 days with no activity. |
ANYONE gonna do anything about this problem and approve his PR? |
This PR is idle because it has been open for 14 days with no activity. |
Can someone please look at this PR? This problem is a constant annoyance. |
This PR is idle because it has been open for 14 days with no activity. |
@Methuselah96 is there any way I can help? I can't even see why the CI failed because it was run so long ago, and I can't trigger it to re-run. Or are you only waiting for the Azure team to look at the PR? |
I tried opening #243 with the exact same changes ( It seems all this PR really needs is attention. |
@DavidBoike Yeah, I'm just waiting on review, not sure what's up with the CI. |
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.
lgtm 🎉
@Methuselah96 How did you reproduce the 404 error (#192) with a long commit message? |
@shubham805 The situation where we run into this is as described here. If you want to reproduce it without squash merge, you can just include a really long multi-line commit message (that includes a one line summary and then a really long description). |
@Methuselah96 The PR looks good. Can you add a dummy commit just to re trigger the pr check pipelines? |
@shubham805 Done, says the workflow is awaiting approval. |
@Methuselah96 There were some issues in the PR check pipeline. We have updated it. Can you again add a new dummy commit, it will pick up the updated workflow file. |
4791c7b
to
4a274aa
Compare
@shubham805 Done, awaiting approval again. |
@shubham805 Are there plans to release this fix? Looking forward to this being resolved. |
@Methuselah96 this is released. Do let us know in case you find any issues. |
We just ran into this on v2 -- appears to maybe not be fixed for every scenario? Using a self-hosted runner. The ZIP deploy URL was 2572 characters long... |
Maybe the change should have been |
This was replaced by #271 anyway, so the code in this PR is no longer used (although I agree it's wrong). |
Should I submit a new issue, about 2572 characters in the URL being too long? #271 assumes that up to 8000 (+ change) characters is acceptable, which from our current experience seems inaccurate. |
Yeah, that would be great, thanks! |
Done: #288 |
Resolves #192.
I considered trimming the commit message to a certain length, but it appears that it would be hard to completely predict how long the message sent to Kudu is since the message contains more than just the commit message. It seems reasonable to just include the first line of the commit message.