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

BREAKING CHANGE: changed generator to an HTML based one #157

Merged
merged 8 commits into from
Feb 6, 2019

Conversation

Berkmann18
Copy link
Member

@Berkmann18 Berkmann18 commented Jan 29, 2019

fix #154

What:
I added and set the HTML generator so that the contributors' list is in HTML/CSS instead of Markdown.
I also tweaked the commit script and added two to test the cli.js module (for production and development).

Why:
Cf. #154.

How:
Replacing the markdown code with HTML + a CSS style block.

Checklist:

  • Documentation
  • Tests
  • Ready to be merged
  • Added myself to contributors table

I'm thinking of adding the style that would slightly change the background colour of entries every two rows but figured it might be worth discussing that first.
Alternatively, having a nice checkboard style (every two rows are bit grey and the rest are white) or something to do with the contributions types/counts (e.g: a colour for coders, one for designers, ...).

I added and set the HTML generator for resolving #154 (which works when testing manually but doesn't
when running the cli in dev mode).

fix #154
@Berkmann18 Berkmann18 changed the title style(generate): changed generator to an HTML based one [WIP] style(generate): changed generator to an HTML based one Jan 29, 2019
Update the contributors test (which wasn't already committed somehow) and tweaked the
`format-contribution-type` test.
README.md Outdated
@@ -1,3 +1,17 @@
<!-- START doctoc generated TOC please keep comment here to allow auto update -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure where this is coming from? Could we disable this?

Copy link
Member Author

@Berkmann18 Berkmann18 Jan 30, 2019

Choose a reason for hiding this comment

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

It might be from the crowdin thing or documentation generator (certainly not the ./cli.js generate).

Either way it's not happening locally (at least within the scope of all-contributors-cli), I'll have a look at the other AC repos and see which one is causing this.

Edit: None of the Yarn scripts on the other repos affected that so I suspect it's from crowdin online or something 🤔.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Ooh, do we need to keep that?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I hope we can remove it 🙏

@jakebolam
Copy link
Collaborator

@Berkmann18 thanks for putting this up. I should be able to review sometime this week!

@jakebolam
Copy link
Collaborator

We should probably add breaking-change in the body of this release commit, to bump the major version

@Berkmann18
Copy link
Member Author

Good point! Yeah, considering some people might be testing the body of the README or the table itself which would technically be a breaking change, so a major version bump would be appropriate.

BREAKING CHANGE: (in 2babe26) The resulting contributors table is in HTML/CSS instead of being in Markdown.
README.md Outdated
| [<img src="https://avatars2.githubusercontent.com/u/17708702?v=4" width="100px;"/><br /><sub><b>Md Zubair Ahmed</b></sub>](https://in.linkedin.com/in/mzubairahmed)<br />[📖](https://github.com/all-contributors/all-contributors-cli/commits?author=M-ZubairAhmed "Documentation") [🐛](https://github.com/all-contributors/all-contributors-cli/issues?q=author%3AM-ZubairAhmed "Bug reports") [💻](https://github.com/all-contributors/all-contributors-cli/commits?author=M-ZubairAhmed "Code") [⚠️](https://github.com/all-contributors/all-contributors-cli/commits?author=M-ZubairAhmed "Tests") | [<img src="https://avatars3.githubusercontent.com/u/6177621?v=4" width="100px;"/><br /><sub><b>Divjot Singh</b></sub>](http://bogas04.github.io)<br />[📖](https://github.com/all-contributors/all-contributors-cli/commits?author=bogas04 "Documentation") | [<img src="https://avatars0.githubusercontent.com/u/15315098?v=4" width="100px;"/><br /><sub><b>João Marques</b></sub>](https://github.com/tigermarques)<br />[💻](https://github.com/all-contributors/all-contributors-cli/commits?author=tigermarques "Code") [📖](https://github.com/all-contributors/all-contributors-cli/commits?author=tigermarques "Documentation") [🤔](#ideas-tigermarques "Ideas, Planning, & Feedback") | [<img src="https://avatars3.githubusercontent.com/u/1192452?v=4" width="100px;"/><br /><sub><b>Andrew Lisowski</b></sub>](http://hipstersmoothie.com)<br />[💻](https://github.com/all-contributors/all-contributors-cli/commits?author=hipstersmoothie "Code") [📖](https://github.com/all-contributors/all-contributors-cli/commits?author=hipstersmoothie "Documentation") [⚠️](https://github.com/all-contributors/all-contributors-cli/commits?author=hipstersmoothie "Tests") | [<img src="https://avatars3.githubusercontent.com/u/1736154?v=4" width="100px;"/><br /><sub><b>Xianming Zhong</b></sub>](https://github.com/chinesedfan)<br />[📖](https://github.com/all-contributors/all-contributors-cli/commits?author=chinesedfan "Documentation") | [<img src="https://avatars2.githubusercontent.com/u/8073251?v=4" width="100px;"/><br /><sub><b>C.Y.Xu</b></sub>](https://github.com/xuchaoying)<br />[💻](https://github.com/all-contributors/all-contributors-cli/commits?author=xuchaoying "Code") |
| [<img src="https://avatars3.githubusercontent.com/u/3680914?v=4" width="100px;"/><br /><sub><b>Dura</b></sub>](https://github.com/chris-dura)<br />[📖](https://github.com/all-contributors/all-contributors-cli/commits?author=chris-dura "Documentation") | [<img src="https://avatars2.githubusercontent.com/u/3534236?v=4" width="100px;"/><br /><sub><b>Jake Bolam</b></sub>](https://jakebolam.com)<br />[🚇](#infra-jakebolam "Infrastructure (Hosting, Build-Tools, etc)") [💻](https://github.com/all-contributors/all-contributors-cli/commits?author=jakebolam "Code") [📖](https://github.com/all-contributors/all-contributors-cli/commits?author=jakebolam "Documentation") [⚠️](https://github.com/all-contributors/all-contributors-cli/commits?author=jakebolam "Tests") | [<img src="https://avatars0.githubusercontent.com/u/8260834?v=4" width="100px;"/><br /><sub><b>Maximilian Berkmann</b></sub>](http://maxcubing.wordpress.com)<br />[💻](https://github.com/all-contributors/all-contributors-cli/commits?author=Berkmann18 "Code") [⚠️](https://github.com/all-contributors/all-contributors-cli/commits?author=Berkmann18 "Tests") [📖](https://github.com/all-contributors/all-contributors-cli/commits?author=Berkmann18 "Documentation") | [<img src="https://avatars2.githubusercontent.com/u/7265547?v=4" width="100px;"/><br /><sub><b>tbenning</b></sub>](https://github.com/tbenning)<br />[🎨](#design-tbenning "Design") |

<style>#emoji-table, #emoji-table td { border: 1px solid #ccc; }</style><table id="emoji-table" cellspacing=0 cellpadding=1><tr><td><a href="https://github.com/jfmengels"><img src="https://avatars.githubusercontent.com/u/3869412?v=3" width="100px;" alt="Jeroen Engels"/><br><sub><b>Jeroen Engels</b></sub></a><br><a href="https://github.com/all-contributors/all-contributors-cli/commits?author=jfmengels" alt="Code">💻</a> <a href="https://github.com/all-contributors/all-contributors-cli/commits?author=jfmengels" alt="Documentation">📖</a> <a href="https://github.com/all-contributors/all-contributors-cli/commits?author=jfmengels" alt="Tests">⚠️</a></td><td><a href="http://kentcdodds.com/"><img src="https://avatars.githubusercontent.com/u/1500684?v=3" width="100px;" alt="Kent C. Dodds"/><br><sub><b>Kent C. Dodds</b></sub></a><br><a href="https://github.com/all-contributors/all-contributors-cli/commits?author=kentcdodds" alt="Documentation">📖</a> <a href="https://github.com/all-contributors/all-contributors-cli/commits?author=kentcdodds" alt="Code">💻</a></td><td><a href="https://github.com/jccguimaraes"><img src="https://avatars.githubusercontent.com/u/14871650?v=3" width="100px;" alt="João Guimarães"/><br><sub><b>João Guimarães</b></sub></a><br><a href="https://github.com/all-contributors/all-contributors-cli/commits?author=jccguimaraes" alt="Code">💻</a></td><td><a href="http://beneb.info"><img src="https://avatars.githubusercontent.com/u/1282980?v=3" width="100px;" alt="Ben Briggs"/><br><sub><b>Ben Briggs</b></sub></a><br><a href="https://github.com/all-contributors/all-contributors-cli/commits?author=ben-eb" alt="Code">💻</a></td><td><a href="https://github.com/itaisteinherz"><img src="https://avatars.githubusercontent.com/u/22768990?v=3" width="100px;" alt="Itai Steinherz"/><br><sub><b>Itai Steinherz</b></sub></a><br><a href="https://github.com/all-contributors/all-contributors-cli/commits?author=itaisteinherz" alt="Documentation">📖</a> <a href="https://github.com/all-contributors/all-contributors-cli/commits?author=itaisteinherz" alt="Code">💻</a></td><td><a href="https://github.com/alexjoverm"><img src="https://avatars.githubusercontent.com/u/5701162?v=3" width="100px;" alt="Alex Jover"/><br><sub><b>Alex Jover</b></sub></a><br><a href="https://github.com/all-contributors/all-contributors-cli/commits?author=alexjoverm" alt="Code">💻</a> <a href="https://github.com/all-contributors/all-contributors-cli/commits?author=alexjoverm" alt="Documentation">📖</a></td></tr><tr><td><a href="https://jerodsanto.net"><img src="https://avatars3.githubusercontent.com/u/8212?v=3" width="100px;" alt="Jerod Santo"/><br><sub><b>Jerod Santo</b></sub></a><br><a href="https://github.com/all-contributors/all-contributors-cli/commits?author=jerodsanto" alt="Code">💻</a></td><td><a href="https://github.com/kevinjalbert"><img src="https://avatars1.githubusercontent.com/u/574871?v=3" width="100px;" alt="Kevin Jalbert"/><br><sub><b>Kevin Jalbert</b></sub></a><br><a href="https://github.com/all-contributors/all-contributors-cli/commits?author=kevinjalbert" alt="Code">💻</a></td><td><a href="https://i.am.charlike.online"><img src="https://avatars3.githubusercontent.com/u/5038030?v=4" width="100px;" alt="tunnckoCore"/><br><sub><b>tunnckoCore</b></sub></a><br><a href="#tool-charlike" alt="Tools">🔧</a></td><td><a href="https://machour.idk.tn/"><img src="https://avatars2.githubusercontent.com/u/304450?v=4" width="100px;" alt="Mehdi Achour"/><br><sub><b>Mehdi Achour</b></sub></a><br><a href="https://github.com/all-contributors/all-contributors-cli/commits?author=machour" alt="Code">💻</a></td><td><a href="https://codsen.com"><img src="https://avatars1.githubusercontent.com/u/8344688?v=4" width="100px;" alt="Roy Revelt"/><br><sub><b>Roy Revelt</b></sub></a><br><a href="https://github.com/all-contributors/all-contributors-cli/issues?q=author%3Arevelt" alt="Bug reports">🐛</a></td><td><a href="https://github.com/chrisinajar"><img src="https://avatars1.githubusercontent.com/u/422331?v=4" width="100px;" alt="Chris Vickery"/><br><sub><b>Chris Vickery</b></sub></a><br><a href="https://github.com/all-contributors/all-contributors-cli/commits?author=chrisinajar" alt="Code">💻</a></td></tr><tr><td><a href="https://github.com/brycereynolds"><img src="https://avatars2.githubusercontent.com/u/1026002?v=4" width="100px;" alt="Bryce Reynolds"/><br><sub><b>Bryce Reynolds</b></sub></a><br><a href="https://github.com/all-contributors/all-contributors-cli/commits?author=brycereynolds" alt="Code">💻</a></td><td><a href="http://www.jmeas.com"><img src="https://avatars3.githubusercontent.com/u/2322305?v=4" width="100px;" alt="James, please"/><br><sub><b>James, please</b></sub></a><br><a href="#ideas-jmeas" alt="Ideas, Planning, & Feedback">🤔</a> <a href="https://github.com/all-contributors/all-contributors-cli/commits?author=jmeas" alt="Code">💻</a></td><td><a href="http://www.spyros.io"><img src="https://avatars3.githubusercontent.com/u/1057324?v=4" width="100px;" alt="Spyros Ioakeimidis"/><br><sub><b>Spyros Ioakeimidis</b></sub></a><br><a href="https://github.com/all-contributors/all-contributors-cli/commits?author=spirosikmd" alt="Code">💻</a></td><td><a href="https://github.com/fadc80"><img src="https://avatars3.githubusercontent.com/u/12335761?v=4" width="100px;" alt="Fernando Costa"/><br><sub><b>Fernando Costa</b></sub></a><br><a href="https://github.com/all-contributors/all-contributors-cli/commits?author=fadc80" alt="Code">💻</a></td><td><a href="https://snipe.net"><img src="https://avatars0.githubusercontent.com/u/197404?v=4" width="100px;" alt="snipe"/><br><sub><b>snipe</b></sub></a><br><a href="https://github.com/all-contributors/all-contributors-cli/commits?author=snipe" alt="Documentation">📖</a></td><td><a href="http://gantlaborde.com/"><img src="https://avatars0.githubusercontent.com/u/997157?v=4" width="100px;" alt="Gant Laborde"/><br><sub><b>Gant Laborde</b></sub></a><br><a href="https://github.com/all-contributors/all-contributors-cli/commits?author=GantMan" alt="Code">💻</a></td></tr><tr><td><a href="https://in.linkedin.com/in/mzubairahmed"><img src="https://avatars2.githubusercontent.com/u/17708702?v=4" width="100px;" alt="Md Zubair Ahmed"/><br><sub><b>Md Zubair Ahmed</b></sub></a><br><a href="https://github.com/all-contributors/all-contributors-cli/commits?author=M-ZubairAhmed" alt="Documentation">📖</a> <a href="https://github.com/all-contributors/all-contributors-cli/issues?q=author%3AM-ZubairAhmed" alt="Bug reports">🐛</a> <a href="https://github.com/all-contributors/all-contributors-cli/commits?author=M-ZubairAhmed" alt="Code">💻</a> <a href="https://github.com/all-contributors/all-contributors-cli/commits?author=M-ZubairAhmed" alt="Tests">⚠️</a></td><td><a href="http://bogas04.github.io"><img src="https://avatars3.githubusercontent.com/u/6177621?v=4" width="100px;" alt="Divjot Singh"/><br><sub><b>Divjot Singh</b></sub></a><br><a href="https://github.com/all-contributors/all-contributors-cli/commits?author=bogas04" alt="Documentation">📖</a></td><td><a href="https://github.com/tigermarques"><img src="https://avatars0.githubusercontent.com/u/15315098?v=4" width="100px;" alt="João Marques"/><br><sub><b>João Marques</b></sub></a><br><a href="https://github.com/all-contributors/all-contributors-cli/commits?author=tigermarques" alt="Code">💻</a> <a href="https://github.com/all-contributors/all-contributors-cli/commits?author=tigermarques" alt="Documentation">📖</a> <a href="#ideas-tigermarques" alt="Ideas, Planning, & Feedback">🤔</a></td><td><a href="http://hipstersmoothie.com"><img src="https://avatars3.githubusercontent.com/u/1192452?v=4" width="100px;" alt="Andrew Lisowski"/><br><sub><b>Andrew Lisowski</b></sub></a><br><a href="https://github.com/all-contributors/all-contributors-cli/commits?author=hipstersmoothie" alt="Code">💻</a> <a href="https://github.com/all-contributors/all-contributors-cli/commits?author=hipstersmoothie" alt="Documentation">📖</a> <a href="https://github.com/all-contributors/all-contributors-cli/commits?author=hipstersmoothie" alt="Tests">⚠️</a></td><td><a href="https://github.com/chinesedfan"><img src="https://avatars3.githubusercontent.com/u/1736154?v=4" width="100px;" alt="Xianming Zhong"/><br><sub><b>Xianming Zhong</b></sub></a><br><a href="https://github.com/all-contributors/all-contributors-cli/commits?author=chinesedfan" alt="Documentation">📖</a></td><td><a href="https://github.com/xuchaoying"><img src="https://avatars2.githubusercontent.com/u/8073251?v=4" width="100px;" alt="C.Y.Xu"/><br><sub><b>C.Y.Xu</b></sub></a><br><a href="https://github.com/all-contributors/all-contributors-cli/commits?author=xuchaoying" alt="Code">💻</a></td></tr><tr><td><a href="https://github.com/chris-dura"><img src="https://avatars3.githubusercontent.com/u/3680914?v=4" width="100px;" alt="Dura"/><br><sub><b>Dura</b></sub></a><br><a href="https://github.com/all-contributors/all-contributors-cli/commits?author=chris-dura" alt="Documentation">📖</a></td><td><a href="https://jakebolam.com"><img src="https://avatars2.githubusercontent.com/u/3534236?v=4" width="100px;" alt="Jake Bolam"/><br><sub><b>Jake Bolam</b></sub></a><br><a href="#infra-jakebolam" alt="Infrastructure (Hosting, Build-Tools, etc)">🚇</a> <a href="https://github.com/all-contributors/all-contributors-cli/commits?author=jakebolam" alt="Code">💻</a> <a href="https://github.com/all-contributors/all-contributors-cli/commits?author=jakebolam" alt="Documentation">📖</a> <a href="https://github.com/all-contributors/all-contributors-cli/commits?author=jakebolam" alt="Tests">⚠️</a></td><td><a href="http://maxcubing.wordpress.com"><img src="https://avatars0.githubusercontent.com/u/8260834?v=4" width="100px;" alt="Maximilian Berkmann"/><br><sub><b>Maximilian Berkmann</b></sub></a><br><a href="https://github.com/all-contributors/all-contributors-cli/commits?author=Berkmann18" alt="Code">💻</a> <a href="https://github.com/all-contributors/all-contributors-cli/commits?author=Berkmann18" alt="Tests">⚠️</a> <a href="https://github.com/all-contributors/all-contributors-cli/commits?author=Berkmann18" alt="Documentation">📖</a></td><td><a href="https://github.com/tbenning"><img src="https://avatars2.githubusercontent.com/u/7265547?v=4" width="100px;" alt="tbenning"/><br><sub><b>tbenning</b></sub></a><br><a href="#design-tbenning" alt="Design">🎨</a></td></tr></table>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Github is rendering like this:
(note the <style> tags)
screen shot 2019-02-02 at 4 53 32 pm

Copy link
Collaborator

Choose a reason for hiding this comment

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

@Berkmann18 this is the only.blocking comment

Copy link
Member Author

@Berkmann18 Berkmann18 Feb 4, 2019

Choose a reason for hiding this comment

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

Yeah.
Note to self: make sure the breaking change note is placed under the right commit when merging.

function createColumnLine(options, contributors) {
const nbColumns = Math.min(options.contributorsPerLine, contributors.length)
return `${_.repeat(nbColumns, '| :---: ')}|`
return `<td>${contributors.join('</td><td>')}</td>`
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

src/generate/index.js Outdated Show resolved Hide resolved
Copy link
Collaborator

@jakebolam jakebolam left a comment

Choose a reason for hiding this comment

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

This approach looks great, and I love the great test coverage!

Thanks for doing this @Berkmann18

Removed the `<style>` block from the generated HTML code as it's redundant on Github (since it's one
of the non-whitelisted tags). The `README.md` was also updated reflecting the breaking changes.
Added `height` to images for avatars, quoted some `<table>` attributes and updated `README.md`
@Berkmann18 Berkmann18 removed the request for review from kentcdodds February 5, 2019 14:34
@Berkmann18
Copy link
Member Author

@tbenning Any thoughts on the design of the table generated?

1 similar comment
@Berkmann18
Copy link
Member Author

@tbenning Any thoughts on the design of the table generated?

Copy link
Collaborator

@jakebolam jakebolam left a comment

Choose a reason for hiding this comment

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

LGTM, lets make sure we add BREAKING_CHANGE to bump major version

@jakebolam
Copy link
Collaborator

jakebolam commented Feb 6, 2019

@Berkmann18 I'm happy to roll this out, and address table updates in a follow up PR

@jakebolam jakebolam changed the title style(generate): changed generator to an HTML based one BREAKING CHANGE: changed generator to an HTML based one Feb 6, 2019
@jakebolam jakebolam merged commit b0c3376 into master Feb 6, 2019
@jakebolam jakebolam deleted the html-table branch February 6, 2019 18:37
@all-contributors-release-bot
Copy link
Member

🎉 This PR is included in version 6.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Switch table generation to HTML
3 participants