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

Cleaner builds #519

Merged
merged 2 commits into from
Oct 22, 2023
Merged

Cleaner builds #519

merged 2 commits into from
Oct 22, 2023

Conversation

luis-pereira
Copy link
Member

No description provided.

It's useful and has a negligible cost.
It's not used for a long long time.
@luis-pereira luis-pereira requested a review from yan12125 October 19, 2023 11:29
@yan12125 yan12125 merged commit 52f7fe8 into master Oct 22, 2023
2 checks passed
@yan12125 yan12125 deleted the cleaner-builds branch October 22, 2023 14:23
@yan12125
Copy link
Member

Thanks!

@luis-pereira
Copy link
Member Author

Squashing into master makes it much harder to find to what commit a change belongs, IMO.
A merge commit is much more helpful.

@yan12125
Copy link
Member

A merge commit is not good as it introduces non-linear history and makes the history more complicated.

@luis-pereira
Copy link
Member Author

A merge commit is not good as it introduces non-linear history and makes the history more complicated.

A squash commit eliminates history. One ends up with a single commit that contains multiple commits. One is not able anymore to say to what commit a change belongs. Squash has it's uses. For features branches merge --no-ff are the best, IMO.
Non linear history is one of the numerous advantages of git over say svn.
20181231230017480

@yan12125
Copy link
Member

Apparently we have some disagreements around project workflow. I guess that will not be a short discussion. Could you open a new ticket?

@luis-pereira
Copy link
Member Author

Apparently we have some disagreements around project workflow. I guess that will not be a short discussion. Could you open a new ticket?

I wouldn't call it disagreements. My view is explained in the post above. There's no need to a new ticket, IMO.
That's your call on how to stuff is merged.

@Chiitoo
Copy link

Chiitoo commented Oct 27, 2023

I don't think I ever did a commit via GitHub, so I'm not sure if there is a way to commit without a merge commit, but also preserve all the commits?

I tend to take the patch of a PR, then 'git am' it and push it via CLI, retaining all the commits and author (possibly picked that up from Gentoo Linux way of doing it, where we don't use GitHub at all to do the actual commits).

I'm not a fan of merge commits myself, but preserving all the commits is of course good (unless there is a good reason to squash them)!

@yan12125
Copy link
Member

yan12125 commented Nov 2, 2023

A squash commit eliminates history. One ends up with a single commit that contains multiple commits. One is not able anymore to say to what commit a change belongs. Squash has it's uses.

In this case, I use squash as those two commits are trivial and closely related. In general, I use rebase to keep individual commits. I can use rebase for future pull requests even when I don't think there is a need to use multiple commits.

For features branches merge --no-ff are the best, IMO.
Non linear history is one of the numerous advantages of git over say svn.

Non linear history is useful in some cases, while I don't see a benefit that outweighs its disadvantage for qtermwidget/qterminal.

I don't think I ever did a commit via GitHub, so I'm not sure if there is a way to commit without a merge commit, but also preserve all the commits?

Rebase preserves author email/name/date and file changes, and merge further preserves committer email/name/date and signature. The latter two are not important for qterminal/qtermwidget IMO.

@luis-pereira
Copy link
Member Author

In this case, I use squash as those two commits are trivial and closely related. In general, I use rebase to keep individual commits. I can use rebase for future pull requests even when I don't think there is a need to use multiple commits.

Maybe they are trivial now... wait two months ;)

inventor2525 pushed a commit to inventor2525/qtermwidget that referenced this pull request May 14, 2024
* Makes some color scheme debug code run

It's useful and has a negligible cost.

* Removes unused debug code

It's not used for a long long time.
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.

3 participants