Skip to content
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

Add binary hash for 13.6.4 #35

Merged
merged 1 commit into from
Mar 1, 2024
Merged

Add binary hash for 13.6.4 #35

merged 1 commit into from
Mar 1, 2024

Conversation

xChickens
Copy link
Contributor

Fixed linting to merge change quicker

@0xdevalias
Copy link
Contributor

0xdevalias commented Mar 1, 2024

The changes look the same, so I assume this PR can probably be closed in favour of that one (which has more in-depth discussion/context/etc)

@0xdevalias 0xdevalias mentioned this pull request Mar 1, 2024
@xChickens
Copy link
Contributor Author

The changes look the same, so I assume this PR can probably be closed in favour of that one (which has more in-depth discussion/context/etc)

I'm just trying to get the change fixed more quickly and not have 4 commits for a 4 line change

@0xdevalias
Copy link
Contributor

0xdevalias commented Mar 1, 2024

I'm just trying to get the change fixed more quickly and not have 4 commits for a 4 line change

Generally speaking, creating a new PR like this is considered creating more noise/spam than continuing on the original; and is unlikely to speed things up.

I can't speak for this repo as I'm not a maintainer, but generally either 4 commits aren't an issue, you could squash/force push them to the same branch/PR anyway, or they could be made a single commit by squash merging that original PR.

It's also generally not ideal to make a PR from a 'main' branch rather than a feature branch:

image

@xChickens
Copy link
Contributor Author

xChickens commented Mar 1, 2024

I'm just trying to get the change fixed more quickly and not have 4 commits for a 4 line change

Generally speaking, creating a new PR like this is considered creating more noise/spam than continuing on the original; and is unlikely to speed things up.

I can't speak for this repo as I'm not a maintainer, but generally either 4 commits aren't an issue, you could squash/force push them to the same branch/PR anyway, or they could be made a single commit by squash merging that original PR.

It's also generally not ideal to make a PR from a 'main' branch rather than a feature branch:

image

For your first point, that would be true if it was a complete duplicate. It isn't, as the other PR has had a lint failure for over a week, which implies the other developer is not that active. If he doesn't correct the PR or come back at all then the PR could remain indefinitely in limbo.

Your second point is fair, but the fact that it took 4 commits and will take another to fix the lint has implications that the developer does not entirely know what they're doing and could easily do a fast forward, rebase, or normal merge, adding all the commits. Extremely common when people aren't experienced with git.

Third point, does it really matter that I didn't use a feature branch? Like it's my own fork of the repo, I can organize it however I want. I don't expect to use main in any way so I could literally rename the main branch to whatever and set it as the default.

@xChickens
Copy link
Contributor Author

Also this issue is specifically affecting my as the max I have access to is 13.6.4. I can't compile the source code since I only have Linux dev environments.

@tulir tulir merged commit 1e1d190 into beeper:main Mar 1, 2024
1 check passed
@0xdevalias
Copy link
Contributor

If he doesn't correct the PR or come back at all then the PR could remain indefinitely in limbo.

@xChickens That's fair. For some reason I thought both PR's were yours; clearly I didn't look closely enough.

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

Successfully merging this pull request may close these issues.

None yet

3 participants