这是indexloc提供的服务,不要输入任何密码
Skip to content

scripts(toolchain): set ASFLAGS=-c #23456

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

scripts(toolchain): set ASFLAGS=-c #23456

wants to merge 1 commit into from

Conversation

stsp
Copy link
Contributor

@stsp stsp commented Feb 24, 2025

Fixes #23102
Untested.

@stsp stsp requested a review from Grimler91 as a code owner February 24, 2025 18:59
@stsp stsp force-pushed the asflags branch 2 times, most recently from fd82250 to 4ae2fdf Compare February 24, 2025 19:04
@stsp
Copy link
Contributor Author

stsp commented Feb 24, 2025

Merging is blocked

    Merge is not an allowed merge method in this repository.
    This branch must not contain merge commits. 

Hmm, what does this mean?
I rebased things on master
a few times - doesn't help.

@TomJo2000
Copy link
Member

We prefer not introduce merge commits in the git history as they unnecessarily muddy up the git history.
So we have disabled merging PRs via git merge.
PRs are instead merged via git rebase, which adds the commits on top of the master branch without an additional merge commit, also keeping the history of the master branch linear.
Or "squashing", which is a form of rebase wherein the commits are combined into a single commit before being added on top of the master branch.

@TomJo2000 TomJo2000 closed this Feb 24, 2025
@TomJo2000 TomJo2000 reopened this Feb 24, 2025
@TomJo2000
Copy link
Member

TomJo2000 commented Feb 24, 2025

Sorry hit the wrong button when posting.

If you want additional information about the different PR merge options see this article in GitHub's documentation section.
https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/incorporating-changes-from-a-pull-request/about-pull-request-merges

@TomJo2000
Copy link
Member

If you are updating your PR branch from the GitHub web interface please make sure to select Update with rebase.
image
We unfortunately cannot turn off the git merge update option for that button, but it creates the same Git history mess as the git merge merge option for PRs.

@stsp
Copy link
Contributor Author

stsp commented Feb 24, 2025

But my branch doesn't have any
merge commits. And it is rebased
on top of master (although master
changes every minute).
So I assume it can be merged
with rebase strategy already.
So what actions are required on
my side to get rid of this blocking?

@stsp
Copy link
Contributor Author

stsp commented Feb 24, 2025

Oh, thanks, trying "Rebase branch".
But it says:

Response.json: Body has already been consumed.

@stsp
Copy link
Contributor Author

stsp commented Feb 24, 2025

I rebased locally (again) and
the button disappeared (again).
But merging is still blocked... :(

@stsp
Copy link
Contributor Author

stsp commented Feb 24, 2025

Switched to "Update with rebase"
and hit Rebase button.
This time it worked.
Still merging is blocked, nothing
changed. :(

@stsp
Copy link
Contributor Author

stsp commented Feb 24, 2025

I can't even demonstrate to
you that the branch is rebased,
as every minute the master
branch changes.

@TomJo2000
Copy link
Member

Oh, thanks, trying "Rebase branch". But it says:

Response.json: Body has already been consumed.

That seems to just be a visual bug.
I noticed it a couple times today.
Still seems to work just fine, maybe GitHub is having an issue with it in the backend.

@TomJo2000
Copy link
Member

I can't even demonstrate to you that the branch is rebased, as every minute the master branch changes.

Yeah we're still catching up on auto-updates since #22994 was just merged and the auto-update workflow reenabled.
It's working through the backlog, but this is a month's worth of auto updates compressed into a day.
So it's a lot busier than usual.

@stsp stsp force-pushed the asflags branch 2 times, most recently from e9637a5 to d3cc5e3 Compare February 24, 2025 20:17
@stsp
Copy link
Contributor Author

stsp commented Feb 24, 2025

Ok but aven after local rebase
and force-push, merging is blocked.
Like it is now.

@TomJo2000
Copy link
Member

I'm not seeing that.
It should also be noted that you are not a Maintainer on this repository and as such do not have permissions to merge pull requests.

As you noted above, this PR is currently untested.
And it will require testing by either you or someone else familiar with the topic before it can be merged.

@stsp
Copy link
Contributor Author

stsp commented Feb 24, 2025

The message is:

Merging is blocked

    Merge is not an allowed merge method in this repository.
    This branch must not contain merge commits. 

But I guess if its not visible to
you, then its a false alarm.

@TomJo2000
Copy link
Member

This is what I'm seeing.
image


If I go to a repo where I do not have maintainer privilges and look at a PR I only get the CI run statistics
image

@stsp
Copy link
Contributor Author

stsp commented Feb 24, 2025

изображение
Here's mine.

@TomJo2000
Copy link
Member

Odd.
Might be an issue they haven't worked out with the new merge UI yet.

@stsp
Copy link
Contributor Author

stsp commented Feb 24, 2025

Correct!
Switching to old UI fixes the problem. :)
Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AS variable is wrongly set
2 participants