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

Revert all layout changes #236

Merged
merged 32 commits into from
Dec 10, 2023
Merged

Revert all layout changes #236

merged 32 commits into from
Dec 10, 2023

Conversation

vidsinghal
Copy link
Collaborator

I will make a PR for this.

This reverts commit 96d52fe.
… in lieu of runtime performance."

This reverts commit 209a90d.
…er boundary optimization."

This reverts commit 0344417.
This reverts commit 4f17ce3.
This reverts commit 32d884b.
This reverts commit bc339a6.
This reverts commit a87fd43.
@ulysses4ever
Copy link
Collaborator

It’s a rather big patch with only (mostly?) reverts. Could you maybe comment on what goal this PR sets?

@vidsinghal
Copy link
Collaborator Author

This PR is to revert all the layout changes that I had introduced to the main branch and instead make a PR request for all these changes so that they can be reviewed by multiple people and to ensure good code is merged.

@ulysses4ever
Copy link
Collaborator

I see. Maybe then the reverts need not be reviewed? I think it'd make more sense to just merge them and then put all the attention we have to the actual changes when they're submitted as a PR.

@vidsinghal
Copy link
Collaborator Author

K, sounds good

@vidsinghal vidsinghal merged commit efd6b60 into main Dec 10, 2023
@ulysses4ever
Copy link
Collaborator

One thing for the future, though: this kind of PR should probably have contained only one committ so that not to pollute the history on the main branch.

@vidsinghal
Copy link
Collaborator Author

Yeah should squash before merging. For next time. A Readme for best practices might be helpful.

@ulysses4ever
Copy link
Collaborator

You're absolutely right! Many OSS projects have a CONTRIBUTING.md document that describes things like this (see e.g. cabal: https://github.com/haskell/cabal/blob/master/CONTRIBUTING.md). Gibbon should absolutely get one too!

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