-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Update GraphQL Explorer in the Tutorial #5024
Conversation
✅ Deploy Preview for redwoodjs-docs ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
Looks awesome 🚀 🎉 Happy Redwood v1.0.0 day 😸 ! |
@jtoar I've realized your (and @simoncrypta's) commit messages often start with lowercase. I'm unaware whether RedwoodJS employs rules of engagement for writing commit messages – but when in doubt i always apply the 7 "golden rules" from this excellent article: https://chris.beams.io/posts/git-commit/ 🧐 |
@Philzen right now we don't have any rules of engagement, just conventions. As to the lowercasing the first word, it's probably my slack tendencies seeping into my commit messages haha. When a PR's merged to main its commits are squashed. But I could definitely put a little more thought into some of my commit messages. We may have considered commit hooks in the past, but it was too much friction at the time. Is that something you'd like to see though? |
@jtoar It's really not about what i'd like to see, i guess. However, since i started adhering to those seven rules a few years ago all my commit histories have become more beautiful and better to read, and i've successfully introduced them to all the teams i've consulted with so far. They're merely a best practice that the Spring team has come up with after years of initial chaos 😉 i didn't realize before you mentioned it that the project is configured to squash all commits on merge. In that case it really doesn't matter that much, apart from PR name to be formatted nicely, preferably according to the 7 rules. Has this setup been a conscious decision? It has a few pro's an cons. Most projects i work on nowadays keep the individual commits and created a separate merge commit that has all the individual commits linked to it. The advantage is, that when there was a single offending commit in the merged feature branch, it's easy to revert just that one without having to revert all other nice functionality that came with it. Disadvantage is: git fanboys like me will nag the hell out of the contributors' commit history until it is only contains perfectly crafted, atomic commits with flawlessly formatted commit messages 😆 |
Yeah it's been configured this way on purpose. While we can't revert only one offending commit of a PR like you mentioned, it's easy for us to revert a PR at least this way, and easier to read the commits in main as individual PRs and backtrack if necessary. We could probably make it a little more sophisticated for sure but it does the job, and is better than what we had before, which was keeping all the commits in a branch. Sometimes over 30! And most with the title "wip" (work in progress) 😅 |
That's what git interactive rebase, fixup, and amend were made for ;) |
I love rebase! But not everyone does. 😅 While rebase feels like a legit superpower, it took me quite some time to learn, and I actually still can't do it without emacs (magit). So it'd just be hard to communicate that we expect everyone to clean up history before merging. Better to just squash it all 🤷 |
Closes #5016. I've still got one more to replace.