Closed ROpdebee closed 3 years ago
I am fine with that, I will just continue following the style of your commit messages as I (roughly) did before. The abbreviations which we are currently using as scope names look sensible to me, they don't have to be unambiguous globally. Although it has a slightly different meaning, what about hide work attrs
instead of collapse work attrs
? Simply work codes
for a combined copy & validate work codes userscript is ok for me, especially as the collapsing of work attributes might be natively supported by MBS in the future.
Given the length restriction for the first line of the commit message, I usually prefer to keep the prefix as short as possible and only include the scope (if at all), especially since I consider most of my commits neither to be "fix" nor "feat", just me changing or enhancing something 😁 So we should definitely define a few short custom(?) types when we start using conventional commits. And I would have to start using synonyms for "to fix" if you do not want all my bug fix commit messages to look like "fix(scope): Fix this and that" as I usually describe them.
Well, there's many different types of changes, like here: https://github.com/conventional-changelog/commitlint/tree/master/%40commitlint/config-conventional so just changing some internals could just be "refactor(scope): yada yada", etc.
In terms of rewording the fix commits, I'd be fine with "fix(scope): this and that" (although arguably it should be two commits, "fix(scope): this" and "fix(scope): that" :P). Looking at the recent history, here's what it could look like:
[work codes] Support other languages on ISWCNet
-> feat(work codes): support other languages on ISWCNet
[caa upload] Fix duplicate image detection on error
-> fix(caa upload): duplicate image detection on error
(or alternatively: fix(caa upload): allow retrying on error
with additional details in the body. That message would better convey the actual fix IMO)In any case, I wouldn't be too strict about it. These commit linter rules look mostly acceptable to me. I won't refuse to merge a PR just because someone wrote fix(scope): Fix stuff
, or forgot to include the right scope*, or even when someone didn't use CC at all (although I'd probably point it out).
Commitizen might be a good idea too.
Anyways, I'll try it out once I start working on enhancements to ECAU, which will be after I'm done with #72
*Although, the latter one would depend on how this integrates with automated build tooling. If the build tools use the scope to determine whether something is up for a release, and lacking a scope causes them to misbehave, then I would probably require a rebase before merging (or just squash and merge and write a commit message myself).
I've made the switch in recent commits, so closing this.
For the record, here's the scopes I've been using lately (mostly for my own reference, because I tend to forget):
scope | project | notes |
---|---|---|
ecau |
mb_enhanced_cover_art_uploads |
Used to be caa upload , somehow I switched to ecau . Might be useful to "expand" this scope if working on a specific provider/seeder (ecau/atisket , ecau/bandcamp )? |
caa dims |
mb_caa_dimensions |
|
split links |
mb_multi_external_links |
|
seed disamb |
mb_qol_seed_disambiguation_comments |
|
caa edits |
mb_supercharged_caa_edits |
|
work codes |
mb_bulk_copy_paste_work_codes |
|
lib |
Anything just affecting @src/lib |
Ones to avoid, but have been used in the past:
qol
: Too broadbuild
: There's a type for that.For what it's worth, I don't prefix the issues and commits with script names or codes.
For commits, I let the file list do it. For the issues I use tags.
I've been using a whole range of different commit message styles, and it makes me cringe a little each time. I should switch to conventional commits instead.
Pros:
Cons:
Conventional commits is often paired with SemVer, but I have no intention of switching over to SemVer. It doesn't make sense for userscripts IMO, and that would lead to lots of issues with auto-updating unless you start with major version 2022. Regardless, there's still other benefits of conventional commits.
Not sure how we should handle scopes though. Currently I'm mostly annotating commits that touch a certain userscript with a sort-of arbitrary prefix (e.g.
[caa upload]
for ECAU), I think it'd make sense to carry that over into the scopes since this is a monorepo of different userscripts, after all. However, I'm not sure what'd be a good scope name for e.g. ECAU.mb_enhanced_cover_art_uploads
would lead to commit headers that are far too long (I don't like the look offeat(mb_enhanced_cover_art_uploads): Add some provider
)ecau
feels like a bad idea because it's not obvious whatecau
stands for, and I've only been using that initialism for just a couple of days in absense of a better shorthandcaa uploads
would probably be the better option, but it kind of feels too "broad" of a name.That's not even mentioning all the other scripts:
mb_blind_votes.user.js
->blind votes
(seems OK)mb_bulk_copy_work_codes.user.js
->work codes
? A bit too broad, IMO.mb_caa_dimensions.user.js
->caa dims
(seems OK too)mb_collapse_work_attributes.user.js
-> Currently usingcollapse work attrs
, but that's a bit long. Also have usedcoll work attrs
butcoll
is ambiguous (could stand for collection too)mb_multi_external_links.user.js
-> No clue, none in use yet.split links
?mb_qol_inline_recording_tracks.user.js
-> None yet.mb_qol_seed_recording_disambiguation.user.js
->seed rec comments
currentlymb_qol_select_all_update_recordings.user.js
-> None yet.mb_supercharged_caa_edits.user.js
->caa edits
currently, a bit too similar tocaa uploads
IMO.mb_validate_work_codes.user.js
->validate work attrs
/validate work codes
currently, too long. It may make sense to eventually merge this script withmb_bulk_copy_work_codes
to create a larger script.@kellnerd, since you're the only other person that's committing to this repo, your opinion matters a lot in this too.