📖 add AMP emoji conventional changelogs #13037

erwinmombay · 2018-01-25T20:36:41Z

added conventional changelog emoji suggestions for PR commits.

feel free to bikeshed.

erwinmombay · 2018-01-25T20:45:04Z

@cramforce should we add something like https://github.com/commitizen/cz-cli to automate this process?

dreamofabear · 2018-01-25T20:47:33Z

Should we add these to PR titles so they're in the squash commits? Or just for commits within a single PR?

erwinmombay · 2018-01-25T20:48:49Z

@choumx looks like subject isn't supported. My original thinking was just sorta require this on subjects instead of all commits but it doesn't seem to work

rsimha · 2018-01-25T21:10:48Z

@erwinmombay @choumx If this is enforced, it should be for PR (and squash commit) titles, since they show up in places like https://github.com/ampproject/amphtml/commits/master and https://travis-ci.org/ampproject/amphtml/builds. I'm not sure it's a good idea to enforce the use of emoji for individual commit messages, as that can get cumbersome very fast.

erwinmombay · 2018-01-25T21:12:08Z

@rsimha-amp titles/subject emoji is not supported in github (see subject above). but it's still useful during the squash.

rsimha

Did someone say bikeshed? :)

rsimha · 2018-01-25T21:12:11Z

.github/PULL_REQUEST_TEMPLATE.md

@@ -16,3 +16,20 @@ Bullet points like
 really help with making this more readable.

 Fixes/Closes/Related-to #1 (enter issue number, except in rare cases where none exists).
+
+It is also helpful to add an emoji before the commit message to identify the kind of work done on a single commit. See the following suggestions below:


Is the idea to select one emoji? Or all that are applicable?

well ideally you should be doing one thing with commits (of course i understand in practice this doesn't happen), so either way.

rsimha · 2018-01-25T21:16:59Z

.github/PULL_REQUEST_TEMPLATE.md

+- :exclamation: - (exclamation) tests
+- :fire: - (fire) P0
+- :rocket: - (rocket) performance improvements
+- :lipstick: - (lipstick) css/styling


Is there anything other than lipstick that says "style"?

couldn't find any. open to other suggestions but other conventional changelog use either this or the diamond

🖍️ (:crayon: seems like a good choice)

rsimha · 2018-01-25T21:17:25Z

.github/PULL_REQUEST_TEMPLATE.md

+- :fire: - (fire) P0
+- :rocket: - (rocket) performance improvements
+- :lipstick: - (lipstick) css/styling
+- :wheelchair: - (wheelchair) accessability


typo: accessibility

rsimha · 2018-01-25T21:18:46Z

.github/PULL_REQUEST_TEMPLATE.md

+- :recycle: - (recycle) refactoring (like moving around code w/o any changes)
+- :building_construction: - (building_construction) infrastructure and tooling
+- :rewind: - (rewind) revert
+- :feelsgood: - (feelsgood) submitting a hacky PR


Should this be a category of its own?

haha no idea. this was just for fun

rsimha · 2018-01-25T21:19:48Z

.github/PULL_REQUEST_TEMPLATE.md

+- :building_construction: - (building_construction) infrastructure and tooling
+- :rewind: - (rewind) revert
+- :feelsgood: - (feelsgood) submitting a hacky PR
+- :construction_worker: - (construction_worker) builds/travis related


Should this be clubbed together with infra and tooling, since they often go together?

I'm ok with that.

rsimha · 2018-01-25T21:20:16Z

.github/PULL_REQUEST_TEMPLATE.md

+- :rewind: - (rewind) revert
+- :feelsgood: - (feelsgood) submitting a hacky PR
+- :construction_worker: - (construction_worker) builds/travis related
+- :put_litter_in_its_place: - (put_litter_in_its_place) removing code


Maybe say "code cleanup" instead of "removing code"?

code cleanup is even more ambiguous though right? like a cleanup could be moving code around like refactoring etc.

Fair enough :)

rsimha · 2018-01-25T21:22:52Z

~~Full list of available github markdown emoji: https://gist.github.com/rxaviers/7360908#file-gistfile1-md~~

Edit: This seems like a more useful list: https://gitmoji.carloscuesta.me/

cvializ

🚲🏚

cvializ · 2018-01-25T21:19:48Z

.github/PULL_REQUEST_TEMPLATE.md

+- :bug: - (bug) bug fix
+- :sparkles: - (sparkles) new feature
+- :exclamation: - (exclamation) tests
+- :fire: - (fire) P0


🔥 is used pretty often for good things, like my mix tape. Maybe 🚨 might be more appropriate?

true. but fire is easier to remember than rotating_light in an actual P0 situation

cvializ · 2018-01-25T21:22:14Z

.github/PULL_REQUEST_TEMPLATE.md

+- :fire: - (fire) P0
+- :rocket: - (rocket) performance improvements
+- :lipstick: - (lipstick) css/styling
+- :wheelchair: - (wheelchair) accessability


The wheelchair is good because :acc autocompletes to it, but it'd be nice if there was something a little more inclusive of all people with accessibility needs rather than only those with physical disabilities.

any suggestions? I was thinking maybe of 🅰️ ?

🅰️ matches but it's a little plain. 👥 (:users / :busts_in_silhouette:) could be good? Maybe we could call the category usability and accessibility? I'm also ok with ♿, it's more a critique of GitHub that :accessibility only autosuggests just the wheelchair image.

cvializ · 2018-01-25T21:24:32Z

.github/PULL_REQUEST_TEMPLATE.md

+- :rewind: - (rewind) revert
+- :feelsgood: - (feelsgood) submitting a hacky PR
+- :construction_worker: - (construction_worker) builds/travis related
+- :put_litter_in_its_place: - (put_litter_in_its_place) removing code


- 🌏 or 🌍 or 🌎 - (:earth_asia: or :earth_africa: or :earth_americas:) i18n

:international autocompletes to one of these

lets go with the global_with_meridians?

cramforce · 2018-01-26T14:44:54Z

So, …, it doesn't work in subjects but then shows up in squash commits? Maybe that is good enough and could be fixed by GH in the future.

cramforce · 2018-01-26T15:34:25Z

.github/PULL_REQUEST_TEMPLATE.md

+- :exclamation: - `:exclamation:` tests
+- :fire: - `:fire:` P0
+- :rocket: - `:rocket:` performance improvements
+- :lipstick: - `:lipstick:` css/styling


🖍 please for gender neutrality.

mrjoro

None of these comments are required changes. :)

mrjoro · 2018-01-26T17:03:59Z

.github/PULL_REQUEST_TEMPLATE.md

+
+- :bug: - `:bug:` bug fix
+- :sparkles: - `:sparkles:` new feature
+- :exclamation: - `:exclamation:` tests


+1 to 🚲🏠 :)

❗️ looks like something that really needs attention (like a high priority issue); is there something more neutral we can use?

I think ✅ (:white_check_mark:) might work here.

mrjoro · 2018-01-26T17:05:08Z

.github/PULL_REQUEST_TEMPLATE.md

+- :rocket: - `:rocket:` performance improvements
+- :lipstick: - `:lipstick:` css/styling
+- :wheelchair: - `:wheelchair:` accessibility
+- :globe_with_meridians: - `:globe_with_meridians:` i18n


add (internationalization) after i18n for people who aren't familiar with the term

mrjoro · 2018-01-26T17:09:12Z

.github/PULL_REQUEST_TEMPLATE.md

+- :recycle: - `:recycle:` refactoring (like moving around code w/o any changes)
+- :building_construction: - `:building_construction:` infrastructure/tooling/builds/CI
+- :rewind: - `:rewind:` revert
+- :feelsgood: - `:feelsgood:` submitting a hacky PR


I associate this term with a different meme so I'd prefer something more neutral if possible.

I don't have a great idea; maybe 🐭 for the game Mouse Trap? :)

haha honestly. I'll just remove it. I just added it for giggles 😃

is from Doom right?

feelsgood is also commonly associated with a certain 🐸 https://www.google.com/search?q=feels+good+man

rsimha

(How) are we going to suggest / enforce this? No one reads our documentation after going through first time setup :)

rsimha · 2018-02-01T20:18:58Z

.github/PULL_REQUEST_TEMPLATE.md

+
+- :bug: - `:bug:` bug fix
+- :sparkles: - `:sparkles:` new feature
+- :exclamation: - `:white_check_mark:` tests


This should read:

:white_check_mark: - `:white_check_mark:` tests

rsimha · 2018-02-01T20:19:36Z

.github/PULL_REQUEST_TEMPLATE.md

@@ -16,3 +16,19 @@ Bullet points like
 really help with making this more readable.

 Fixes/Closes/Related-to #1 (enter issue number, except in rare cases where none exists).
+
+It is also helpful to add an emoji before the commit message to identify the kind of work done on a single commit. See the following suggestions belowf


s/belowf/below:/

rsimha · 2018-02-01T20:21:10Z

.github/PULL_REQUEST_TEMPLATE.md

+- :recycle: - `:recycle:` refactoring (like moving around code w/o any changes)
+- :building_construction: - `:building_construction:` infrastructure/tooling/builds/CI
+- :rewind: - `:rewind:` revert
+- :put_litter_in_its_place: - `:put_litter_in_its_place:` deleting code


I suggest capitalizing all the phrases, like:

Bug fix New feature Tests

etc.

erwinmombay · 2018-02-01T20:33:10Z

@rsimha-amp i don't think we're looking to enforce this. but there are tools like https://github.com/commitizen/cz-cli and conventional-changelog that does

rsimha

Gotcha. LGTM after a few more nits.

rsimha · 2018-02-01T20:34:16Z

.github/PULL_REQUEST_TEMPLATE.md

+- :white_check_mark: - `:white_check_mark:` Tests
+- :fire: - `:fire:` P0
+- :rocket: - `:rocket:` Performance improvements
+- :crayon: - `:crayon:` css/styling


CSS / styling (capitalization, spacing)

rsimha · 2018-02-01T20:34:37Z

.github/PULL_REQUEST_TEMPLATE.md

+- :rocket: - `:rocket:` Performance improvements
+- :crayon: - `:crayon:` css/styling
+- :wheelchair: - `:wheelchair:` Accessibility
+- :globe_with_meridians: - `:globe_with_meridians:` i18n (internationalization)


i18n (Internationalization) (capitalization)

rsimha · 2018-02-01T20:35:02Z

.github/PULL_REQUEST_TEMPLATE.md

+- :globe_with_meridians: - `:globe_with_meridians:` i18n (internationalization)
+- :book: - `:book:` Documentation
+- :recycle: - `:recycle:` Refactoring (like moving around code w/o any changes)
+- :building_construction: - `:building_construction:` Infrastructure/Tooling/Builds/CI


Infrastructure / Tooling / Builds / CI (with spaces)

* 📖 add AMP emoji conventional changelogs * apply recommendations

googlebot added the cla: yes label Jan 25, 2018

erwinmombay force-pushed the add-pr-emoji branch 3 times, most recently from edf3868 to d2ee37d Compare January 25, 2018 20:39

erwinmombay requested review from cramforce, mrjoro and jridgewell January 25, 2018 20:39

erwinmombay force-pushed the add-pr-emoji branch 2 times, most recently from 138dd63 to dbfd54f Compare January 25, 2018 20:42

erwinmombay changed the title ~~add AMP emoji conventional changelogs~~ 📖 add AMP emoji conventional changelogs Jan 25, 2018

erwinmombay changed the title ~~📖 add AMP emoji conventional changelogs~~ 🐛 📖 add AMP emoji conventional changelogs Jan 25, 2018

erwinmombay changed the title ~~🐛 📖 add AMP emoji conventional changelogs~~ add AMP emoji conventional changelogs Jan 25, 2018

erwinmombay force-pushed the add-pr-emoji branch 2 times, most recently from 2b28e89 to 05d6cee Compare January 25, 2018 20:44

erwinmombay force-pushed the add-pr-emoji branch from 05d6cee to 65a3bad Compare January 25, 2018 20:46

erwinmombay force-pushed the add-pr-emoji branch from 65a3bad to 64642a0 Compare January 25, 2018 20:47

erwinmombay force-pushed the add-pr-emoji branch 2 times, most recently from 697c7eb to 2356d67 Compare January 25, 2018 20:56

erwinmombay changed the title ~~add AMP emoji conventional changelogs~~ 📖 add AMP emoji conventional changelogs Jan 25, 2018

rsimha reviewed Jan 25, 2018

View reviewed changes

cvializ reviewed Jan 25, 2018

View reviewed changes

erwinmombay force-pushed the add-pr-emoji branch from 2356d67 to 4ddd0fb Compare January 25, 2018 21:51

📖 add AMP emoji conventional changelogs

4e97621

erwinmombay force-pushed the add-pr-emoji branch from 4ddd0fb to 4e97621 Compare January 25, 2018 21:52

cramforce reviewed Jan 26, 2018

View reviewed changes

mrjoro reviewed Jan 26, 2018

View reviewed changes

rsimha suggested changes Feb 1, 2018

View reviewed changes

erwinmombay force-pushed the add-pr-emoji branch 3 times, most recently from 5d7c95c to ea4db14 Compare February 1, 2018 20:25

rsimha approved these changes Feb 1, 2018

View reviewed changes

erwinmombay force-pushed the add-pr-emoji branch from ea4db14 to 3f914c4 Compare February 2, 2018 18:41

apply recommendations

1a65459

erwinmombay force-pushed the add-pr-emoji branch from 3f914c4 to 1a65459 Compare February 2, 2018 18:44

erwinmombay merged commit 910d343 into ampproject:master Feb 5, 2018

ampprojectbot added the PR use: In Stable label Feb 14, 2018

rsimha mentioned this pull request Feb 20, 2018

📖 Make the AMP PR template more readable #13568

Merged

protonate pushed a commit to protonate/amphtml that referenced this pull request Feb 26, 2018

📖 add AMP emoji conventional changelogs (ampproject#13037)

33cf3ba

* 📖 add AMP emoji conventional changelogs * apply recommendations

RanAbram pushed a commit to RanAbram/amphtml that referenced this pull request Mar 12, 2018

📖 add AMP emoji conventional changelogs (ampproject#13037)

b1e6c01

* 📖 add AMP emoji conventional changelogs * apply recommendations

protonate pushed a commit to protonate/amphtml that referenced this pull request Mar 15, 2018

📖 add AMP emoji conventional changelogs (ampproject#13037)

51dbb6d

* 📖 add AMP emoji conventional changelogs * apply recommendations

📖 add AMP emoji conventional changelogs #13037

📖 add AMP emoji conventional changelogs #13037

Conversation

erwinmombay commented Jan 25, 2018 • edited

erwinmombay commented Jan 25, 2018

dreamofabear commented Jan 25, 2018

erwinmombay commented Jan 25, 2018 • edited

rsimha commented Jan 25, 2018

erwinmombay commented Jan 25, 2018 • edited

rsimha left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rsimha commented Jan 25, 2018 • edited

cvializ left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cvializ Jan 25, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cvializ Jan 25, 2018 • edited

Choose a reason for hiding this comment

cvializ Jan 25, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cramforce commented Jan 26, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrjoro left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cvializ Jan 29, 2018 • edited

Choose a reason for hiding this comment

rsimha left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erwinmombay commented Feb 1, 2018

rsimha left a comment

Choose a reason for hiding this comment

rsimha Feb 1, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rsimha Feb 1, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erwinmombay commented Jan 25, 2018 •

edited

erwinmombay commented Jan 25, 2018 •

edited

erwinmombay commented Jan 25, 2018 •

edited

rsimha commented Jan 25, 2018 •

edited

cvializ Jan 25, 2018 •

edited

cvializ Jan 25, 2018 •

edited

cvializ Jan 25, 2018 •

edited

cvializ Jan 29, 2018 •

edited

rsimha Feb 1, 2018 •

edited

rsimha Feb 1, 2018 •

edited