Why Doesn't Curl Merge Pull Requests Properly??
Vložit
- čas přidán 23. 06. 2024
- Whilst git by itself is a great tool by itself, tools like Github tend to force you into a certain kind of development model which simply doesn't work for a lot of projects out there, Curl being one of them.
==========Support The Channel==========
► Patreon: brodierobertson.xyz/patreon
► Paypal: brodierobertson.xyz/paypal
► Liberapay: brodierobertson.xyz/liberapay
► Amazon USA: brodierobertson.xyz/amazonusa
==========Resources==========
Curl Blog Post: daniel.haxx.se/blog/2024/06/1...
Initial Request: github.com/isaacs/github/issu...
Current Request: github.com/github/feedback/di...
=========Video Platforms==========
🎥 Odysee: brodierobertson.xyz/odysee
🎥 Podcast: techovertea.xyz/youtube
🎮 Gaming: brodierobertson.xyz/gaming
==========Social Media==========
🎤 Discord: brodierobertson.xyz/discord
🐦 Twitter: brodierobertson.xyz/twitter
🌐 Mastodon: brodierobertson.xyz/mastodon
🖥️ GitHub: brodierobertson.xyz/github
==========Credits==========
🎨 Channel Art:
Profile Picture:
/ supercozman_draws
#Github #Linux #Git #OpenSource #FOSS
🎵 Ending music
Track: Debris & Jonth - Game Time [NCS Release]
Music provided by NoCopyrightSounds.
Watch: • Debris & Jonth - Game ...
Free Download / Stream: ncs.io/GameTime
DISCLOSURE: Wherever possible I use referral links, which means if you click one of the links in this video or description and make a purchase I may receive a small commission or other compensation. - Věda a technologie
that curl blog post link looks suspiciously wayland protocol shaped
That's fucking hilarious
LOL, LMAO even
"bug: wayland bikeshedding taking up more brain time than expected"
Wayland shill never disappoints
xdg-curl-v1 confirmed?
why tf would you write a browser extension when UBLOCK ORIGIN IS RIGHT THERE and it's ENTIRE JOB is removing stuff
similarly there's already stuff like stylus and greasemonkey and whatnot, letting you apply userstyles and userscripts on websites, that'd also let you hide the button
People see "ubo" and think "adblocker" and so using it outside of that purpose doesn't cross their mind unless/until they're told "by the way ubo is actually more than just an adblocker".
@@NightKev yeah just like sponsorblock is more than just a sponsor blocker, but most people don't know
Reminds me of my last job where I used greasemonkey to remove the merge button from pull request screen in gitlab so I won't accidentally merge it
(when merging gitlab first merges the target branch into source branch and then merges source back to target, but our manager required branches to stay clean so he could pick and choose what should be merged into final release).
@@felixfourcolor and ajay still doesn't add full video filler labels, despite their obvious utility
LINUS WAS RIGT
LINUS WAS RIGT
who's rigt
Leenoz Always Right....vree sofwear
LINUS WAS RICH
LINUS WAS RICH
9:45 throwing shade at apple users in the politest way possible
Ahh so you prefer Google's MS 2.0 tactics of embrace and extend the RCS protocol to lock people out instead of Apple's walled garden if you want it, you get our stuff. Got it.
This could all be solved with policies to enforce conventional commits, and maybe squash merge.
Maybe some mini scripts that can run post-commit / post-merge that you can use to automatically change status.
Exactly, I thought when you squash merge then whoever initiated the merge can the create message however he sees fit right?
edit: eventually got to that part in the video
It's a lot of work to fight with the GitHub UI when Git's interactive CLI is honestly 🐐ed for this exact use-case.
a game i play, called barotrauma, actually has a public and private repo, so they close pr's on the public repo after merging it privately
Key words like 'addresses' and 'closes' are only applied when the commits are merged into the main branch.
Okay, my nerd brain heard "force push" and immediately went to an image of Torvalds as a Jedi force pushing code into a repo. lol
If you've never had the chance to do a force-push before... that is _exactly_ what it feels like. Even better when you're bypassing branch protections because you're the repo administrator.
Excellent video 👍 Thank you 💜
you don't need a whole extension to hide something from a website, you can just right click > block with ublock origin. I do this all the time to hide stuff that annoys me on websites, which includes half of the youtube UI
Aaaaaa. I really loved the prior title!
The link in the description is wrong lol
Thought you would have curled up into a ball at the end of the wayland icons epic!
you don't even need to learn how to write a browser plugin. All you need is to install a css browser plugin like stylus and just add `display: none;` to the rule selecting the merge button.
You can do this with uBlockOrigin too
Could have gone with "curl up into a tarball"
I think the easiest fix would be to have an "edit commit message before merge" button, so that the git history definitely matches the pull request
i.e. I could mark a random commit as "merges pull request xyz" even though it doesn't
Editing the commit message elides that issue entirely
Github does give functionality to let the owner of the repo like curl in this example, to do changes in the fork directly. Where basically they can do the commit name changes in the fork itself and accept the PR normally into the curl repo.
Only if the submitter allows it. There's a checkbox that can enable/disable
I'm aware but it doesn't integrated nicely like the other tooling, like commenting on code snippets
brodie sourcehut shill arc??
The "hide merge button in these repos" could also be added to the refined GitHub browser extension.
This is not advice for the curl project, they’re very big, but if you’re starting something new there are lots of better ways to enforce commit message formats. Contrarianism against Github doesn’t make your software better.
Commitlint is available in NodeJS and python, and probably other languages. You can put that into the Ci pipeline to make sure or use a pre-commit hook. You can also enforce squashing and don’t approve unless the PR title is what you want. They also manually make their release notes, which github can actually do now pretty nicely for small projects (based on merged PRs).
From my experience pre-commit hooks don't work correctly.
They are nice if you want to trigger something external, but not for scanning the code/commit.
I had a pre-commit hook made by someone else that was supposed to fix code style (change tabs to spaces etc.) and for some unknown to me reason it was grabbing the wrong code and adding stuff that I explicitly told it not to commit (I am using line by line staging so I can split my changes into multiple commits with its own comments).
GitHub is not an authority in using git. Following their nomenclature doesn't make software better either
Pre-commit is 🐐ed in my experience. The only issue I've had is when I've forgotten to install it 😅, but you can actually enforce it (and even do autofixes) through CI. To @@hubertnnn's point, you do have to properly vet your hooks, but ones I rely on have been rock solid and well-maintained.
the ad which penetrates the ad block extensions.... microshaft azure
I swear GitHub also marks merged when using "merges ###" but apparently I'm wrong. :/
11:20 Oh hey, it's exactly what I thought it already did. I ALSO thought "fixed"/"fixes" already does this. :/
GH "magic words" have been super flaky for me lately. It's probably a tenses issue ("resolve" vs. "resolves") or I need to check the cheatsheet, but honestly my projects are small enough that the extra click is no biggie.
We solve this with a release script and tags on PRs to categorize them. We always squash when we merge and we can change the commit message for that squashed commit.
One thing I disagree is that there shouldnt be a merged #id command. The PR that is merged is not the one that is on github, so you either need to completly over write it or to say it is merged even if something else was merged.
That said, having separate, purple status like "accepted" doesnt seem to have those issues.
What is GitLab like in comparison to GitHub.
Can these things be implemented through a policy / configuration option ?
ci could also add a tag for merged to the pr when it's closed...
Why "Closes" keyword would even close the pull request when that pull request is being merged?
Closes keyword should always switch PR to merged status, since Closed status means PR was rejected, while Merged means it was accepted.
If PR code is being merged then obviously PR is accepted.
At worst case they should add Rejects keyword that changes PR status to closed, but I doubt there is even a use case for merging PR while changing its status to Closed.
That commit doesn't have to be a merge. For example someone could make a PR for something because they didn't understand how something works and thought it was useful. Someone could make a commit to improve the documentation to explain why such a feature wouldn't make sense and close the PR.
Interesting. I do all my merges locally, at a command line, then push to trunk after tests have passed... and the PRs show up as merged, not closed. I wonder why.
If you pull the exact commits as-is from a PR and merge them with a merge commit then push, it'll do that.
If you edit the commit messages (which is what cUrl is doing), the hashes won't match and github won't detect that the PR has been merged
@@triffid0hunter That makes sense. I can see how people would run into problems by making destructive edits to history. The commits aren't marked as merged because they were literally not merged, but rather, rewritten. That sounds like the correct behavior.
I wonder if you can do this on gitlab?
I do this a lot on cmangos repos and there is no way around it. There is no keyword for commit "merged" that would show the web merged icon. Also if you keep a clean trunk with squashing, you generally dont want the autosquashed commit message.
In my opinion, all the changes in a pull request should all be related. If you have additional changes that aren't related, you should submit another pull request for those changes.
How does Gitlab handles this situations?
Basically the same
Squash Commits. The same way Github does. Force push is not an idiomatic way to use git and is incredibly dangerous.
@@KraustIt doesn't need to be a force-push, though. It should be an interactive rebase of the PR branch onto the HEAD and a regular push. I didn't quite understand why it's not.
The irony in spelling right wrong was not lost on me.
I need to see about making a Windows-related contrib to the sdl github. Would anyone know to do so? No. Unless they're objectively insane as I am. Insane enough to compile OpenRA then SDL2 on Windows 11... on a SQ1 Surface Pro X. That's right, I was an early adopter of the Surface Pro X. You can have what opinions on this you want. I want to see x86 dethroned, and I don't give a damn who does it!
Use the math style tag bug to hide the merge button 😅
curl uses the fitgirl theme 👀
But there's also the Git Hub CLI
I thought this video was going to be about some bug in curl's code that if you make a curl request to the github api that it would fuck up. The actual video makes a lot more sense than what I though lol
If there is a f-up and there are two well-established projects, one of them being GitHub, the fault usually lies with GitHub.
Daniel of curl and Wayland videos is about 80% of your video content ? 🙂 (not that I mind, just a pattern I noticed)
9:02 the irony of Git's creator (Git wasn't known as .... euh the easiest tool to work with) complaining about usability-related issues. 🙂 Anyway, yes, git should be leading.
Interestingly, it seems that the same issues are present on GitLab, at least I didn't find a way to comment on commit message.
Developers developers developers! Developers developers developers!
Do merge requests on Mir get mirged? 😳
Personally I find squash+merge to be the best option. The reasons for why it doesn't work for someone seem superfluous
squash+merging a large pr with a bunch of commits makes it impossible to do a proper git bisect
@@m4rch3n1ng why not switch to the branch and do bisect there after pinning down the issue to the PR?
I don't like that if he merges something from someone else it shows that he did it in the git log
Not to say that isn't a problem, but it's a GitHub problem. Fun fact: Git actually stores two identities (and timestamps) per commit! One for the "author" (who actually wrote the code) and one for the "committer" (the last person to write the commit in question, for example by rebasing or `git am`-ing). GitHub, iirc, displays the latter, but you can easily customize the `git log` format on your system to include or even prioritize the former!
@@lritzdorfIIRC GitHub shows both names. It also lets you add more names with a Co-Authored-By tag.
If you look at the log it actually shows who committed the code and who authored the code as 2 separate things
Brodie, do you think you'll hit 100k subscribers by end of year... Lesgo!
If you asked me a few months ago, I would have said no but things have sped up recently so I'm not too sure anymore. It would be awesome if it happened
I agree with curl - when you have a project tgat has lots of one-time commiters its just soooo much easier to manually merge changes so that you can ensure it meets project standards without having lots of back and forth uselessly educating people about those project standards for their single one time commit
Why is it always curl?
Daniel runs a blog and likes to talk about the internals of the project
@@BrodieRobertson I was trying to joke around a bit about whenever there is a larger issue ongoing, curl seems to always be related somehow.
Just squash and merge..
It lets you change the commit message
I agree though, Github shouldn't dictate how git should be used.
Have you ever noticed that GitHub doesn't have a fast forward merge option? It's ridiculous that GitHub the #1 git repo service does not support a fundamental feature of git.
This for example forces me to use rebase onto the development branch but when I want to get my master branch up to speed I have to use a merge commit causing "master is x commits ahead of development" even though it is entirely the same as far as the code is concerned.
Strongly disagree. I make my commits atomic (which is idiomatic in Git, nevermind GitHub). (I do use e.g. git add -p, interactive rebase, etc to achieve this.) The commits are much more granular than PRs. It helps to understand the steps involved, and facilitates troubleshooting and cleaning up the history.
There is no sense discarding someone else's work like that.
Squash and merge seems like something for people that use git begrudgingly because they were forced to. Missing the point of it
Not sure where to begin but this feels like a wrong take on the issue. The problem seems like "garbage in" that you fix manually in a new commit with comment to close the pull request instead of merging it.
The solution should not be to help fix the close status of the pull request, it should be to help have a style guide for commit and pull requests in git, or am i completely wrong?
If you github pr side setup a style guide for how issues are named or, even better, a commit prehook that checks commit message contains correct style you won't get "garbage in".
I understand that prehooks are client self setup and not something the repo configures but this should at least be a validation pipeline that comments the code of conduct if commits/pr title does not follow style guide.
Don't get me wrong, I love more features in github and customablity is most times good.
But to me this seems to be a bandaid to the real problem.
Love your videos!
Linus is right, he just has no tact about explaining why, he is not a PR guy for sure lol
Took 13 minutes to get to Squash Commits which is how "normal" projects do this. I have been yelling at the screen for 13 minutes.
"Multiple issues per PR/MR"
No. That is not how software should be written. They are doing it wrong. Create a process. Follow the process. If your process is not idiomatic to the tool you're using (in this case Github) make it idiomatic or don't use that tool.
It is idiomatic to the tool, to git, github adds arbitrary changes on top of that. It's perfectly fine to just use github as a bug tracker and remote and then do all the merging externally
Strongly disagree. I make my commits atomic (which is idiomatic in Git, nevermind GitHub). (I do use e.g. git add -p, interactive rebase, etc to achieve this.) The commits are much more granular than PRs. It helps to understand the steps involved, and facilitates troubleshooting and cleaning up the history.
I'd be frustrated if that work were thrown away.
Fork the Git.
Curl is everything but a big project. It is 900 files of which half header files. Having worked as software developer more than half of my life, this constitutes barely a medium project. Big projects are 10 times this (around 10k), and huge projects are the caliber of the Linux kernel (50k files).
As Linus said - git is not an SCM tool, it's a tool that can be used to create an SCM.
git flow is kind of an SCM, TFS is an SCM, GitHub is an SCM.
An SCM describes not a mechanism for tracking code changes but also a workflow and processes for how code changes are propagated from developers to product. Notably a lot (all?) of FLOSS git competitors are SCMs in their own right, such as Subversion or Mercurial - so Linus has only himself to blame for doing a half-assed job and letting the community run with it...
Disagree. I often don't use the GitHub interface at all, and regularly rewrite commit descriptions, rebase, and force push to the PR. No issues with GitHub detecting a merge if you do it correctly. Github detects a merge when the commit history in the target branch matches the PR.
Either force-push local changes to the PR branch before pushing that to master/main, or change the target branch to the secondary branch to merge it, rebase, and then merge the secondary branch into master/main.
Also it sounds like GitHub's squash merge button is everything curl wants for single commit PRs. It will condense all commits in the PR into one, and you can rewrite the commit description from the interface.
You can't always force push to the PR branch. Say that I am contributing to a project, but I'm not a maintainer, so I can't push to the repo directly. I'm therefore going to open a PR from my fork. The project maintainers won't be able to force push to my fork's PR branch (they can of course clone the repo and push to the branch locally but then the commit hashes wont line up with what github sees and it won't auto-detect the merge) just like I can't push to a branch in the official project repo. They could _ask me_ to make a force push to update the commit names, but that adds time and complexity dealing with other people.
@@manchmalscott When you create a PR in GitHub, the owner of the repository that you made the PR to has force-push rights to your fork's branch. In practice, I regularly force-push to contributor's PRs. In either case, if you want the PR to be merged as one commit with a custom description, you can also use GitHub's squash merge button.
@@mmstickwhen creating a pr one can uncheck the checkbox that allows that
@@joshix833 I've yet to encounter anyone who would do that in practice. But if I did, I could always create a secondary branch to merge the PR to, rebase, and then merge that onto the main branch.
Its discussed at the end of the video why it does not work for them
Have these people never heard of squash commits? The commit history of the PR should never be included in the final commit timeline after a merge anyway, thats just asking for a bad time. I've never seen any big project being run this way.
Yeah this whole thing is weird. Click the squash button and make the commit message match your style guide lol
Squash was addressed in the post, the issue with squashing is you destroy the commit history which isn't always desired
Copying my relevant reply here...
Strongly disagree. I make my commits atomic (which is idiomatic in Git, nevermind GitHub). (I do use e.g. git add -p, interactive rebase, etc to achieve this.) The commits are much more granular than PRs. It helps to understand the steps involved, and facilitates troubleshooting and cleaning up the history.
I'd be frustrated if that work were thrown away.
No, it should, for the purposes of granularity and debugging.
Rebase is the best
First
Kek
why r you in every brodie video's comments
@@ardishco gee, I wonder why 🤔