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

Remove the last of the old kind attributes #621

Closed
wants to merge 75 commits into from

Conversation

NogginBops
Copy link
Contributor

This PR fixes the last stuff that was left in in #534. Basically this PR replaces and removes the last CheckedFloat and CheckedInt32 kinds (as well a few others kinds that where leftovers and never described formally).

I thought I had fixed these things but it turned out that I had some +1 year old commits in this branch that I never opened a PR for.

cc @SunSerega

NogginBops and others added 22 commits October 26, 2022 13:39
Co-authored-by: Sun Serega <[email protected]>
… generic name but only one usage, should kind=Clamped be used?
@NogginBops
Copy link
Contributor Author

I think I might have messed up the git stuff as I think some of these commits are part of #534. Not sure if I can fix this in any good ways.

@SunSerega
Copy link
Contributor

SunSerega commented Jul 16, 2024

I think this is because #534 was squash-merged.
In other words, an equivalent commit was added to the main branch, so this PR is compatible, but the commits of #534 themselves are not on main - so they are shown in changes.

One way to fix this is to rebase the more-kind-changes branch onto the main branch.
Rebase would make the current main - the origin of the changes, as if you started from scratch and cherry-picked every commit of this PR in the same order, discarding the now no-op commits (because they were already merged into main).

Though rebase might be messy in this case.
It might be easier to reset --hard the more-kind-changes branch onto the main branch, and manually cherry-pick the only relevant commits from the remote version of this branch.

Either way, you'd need to push --force (or push --force-with-lease) to fix how it looks on GitHub.
Before forcing new (alternative) history onto GitHub, I recommend that you diff the remote and the local (rebased) states of the more-kind-changes branch to make sure there aren't any changes you were unaware of.

@NogginBops
Copy link
Contributor Author

#622 fixes the git history. Used git cherry-pick in the specific commits that where missing in #534 into a new branch based on main.

@NogginBops NogginBops closed this Jul 16, 2024
@NogginBops NogginBops deleted the more-kind-changes branch July 16, 2024 16:07
@SunSerega
Copy link
Contributor

SunSerega commented Jul 16, 2024

There are 3 changes in gl.xml between the last 2 PRs.
Well, the first 2 look reasonable (though it's weird why it was like that in this PR), but the third one is a kind="Range[0; x]" => kind="CheckedInt32" change.

@NogginBops
Copy link
Contributor Author

Will fix!

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.

2 participants