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

Add 'bias', 'obfuscate' and 'tidy' #140

Merged
merged 1 commit into from
Jul 25, 2024

Conversation

faern
Copy link
Contributor

@faern faern commented Jul 25, 2024

Another small batch of allowed verbs that we have ended up using since adopting this CI check! Following in the footsteps of #130 and #131 :)

@faern faern force-pushed the add-bias-obfuscate-tidy branch from 4d7856c to 03b8047 Compare July 25, 2024 07:14
Copy link
Owner

@mristin mristin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @faern !

@mristin mristin changed the title Add 'bias', 'obfuscate' and 'tidy' as allowed verbs Add 'bias', 'obfuscate' and 'tidy' Jul 25, 2024
@mristin
Copy link
Owner

mristin commented Jul 25, 2024

@faern can you please fix the commit message? (See the failing CI.)

@faern
Copy link
Contributor Author

faern commented Jul 25, 2024

@mristin

@faern can you please fix the commit message? (See the failing CI.)

I already have... Please check the commits tab. I dunno why the CI did not re-run

@faern faern force-pushed the add-bias-obfuscate-tidy branch from 03b8047 to 5141858 Compare July 25, 2024 07:39
@faern
Copy link
Contributor Author

faern commented Jul 25, 2024

I recommited the same thing and force pushed, just to trigger the CI. Should be fine now?

@faern
Copy link
Contributor Author

faern commented Jul 25, 2024

Oh, lol. It was complaining about the length of the github PR title? 😂 My god. I have been so confused about that before.

Why do you enforce that to be so short? I guess that's a more ideological question. The only reason I want to limit the length of git commit messages is so they display nicely when rendered as a tree in my terminal. For the github PR title and description none of those limitations apply.

@mristin
Copy link
Owner

mristin commented Jul 25, 2024

Why do you enforce that to be so short?

I think I couldn't really separate the two in the code as far as I can remember. I'm open to changing that behavior -- please feel free to open a new issue!

@faern
Copy link
Contributor Author

faern commented Jul 25, 2024

But we are using this CI action and only limit git commit messages, not github PR content. It just works? 🤷 😂
https://github.com/mullvad/mullvadvpn-app/blob/main/.github/workflows/git-commit-message-style.yml#L25-L35

@faern
Copy link
Contributor Author

faern commented Jul 25, 2024

However, that is completely unrelated to this PR. Feel free to merge the extra verbs :)

@mristin
Copy link
Owner

mristin commented Jul 25, 2024

But we are using this CI action and only limit git commit messages, not github PR content. It just works? 🤷 😂 https://github.com/mullvad/mullvadvpn-app/blob/main/.github/workflows/git-commit-message-style.yml#L25-L35

I think the event is crucial -- if you run it against pull request, the body will be the pull request description. I'm not really sure anymore, though.

@mristin mristin merged commit a816920 into mristin:master Jul 25, 2024
3 checks passed
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