Skip to content

Mention using GGG on github.com/git/git #5

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

Merged
merged 10 commits into from
Mar 20, 2020

Conversation

phil-blain
Copy link
Contributor

As discussed in gitgitgadget/gitgitgadget#146, here is a small update to the GGG homepage to mention that it can also be used on github.com/git/git.

This PR also adds the following to the homepage:

  • Add a couple of links to other resources
  • Mention the CC syntax
  • Add a section mentioning GGG's features and which work on GGG's git fork vs github.com/git/git

@dscho @webstech

Note: the table and its CSS was generated using an online HTML table generator, which I tried to clean up a bit. Also if we'd rather not use emoji in the table we could use "yes"/"no".

Don't hesitate if you think of other features that should be mentioned or that are not available on github.com/git/git.

@phil-blain
Copy link
Contributor Author

I've activated GitHub pages on my fork so that the formatted result can be reviewed:
https://phil-blain.github.io/gitgitgadget.github.io/

@dscho
Copy link
Member

dscho commented Mar 17, 2020

Looks good!

Two comments:

  • the table and the Cc: example seem too small when I look at it on a phone screen
  • the table entries would read better to me in the 3rd person, e.g. "adds a PR comment when the patches made it to pu, next, master or maint

What do you think?

@webstech
Copy link

Good idea.

  • The <tt> tag is deprecated and does not look very good on an iPad (both Chrome and Safari look bad). Actually, it looks like there is an alignment issue in chrome on a pc as well (too low).
  • The CC: example has tabs which are causing a positioning problem. Use html/css for positioning? From a phone:
    image
  • GitGitGadget will use these as the cover letter. Not sure where else the cover letter is mentioned but there isn't really one now for single commit PRs. I understand it is included in the email and maybe it just needs to say that and not specifically the cover letter.

@dscho
Copy link
Member

dscho commented Mar 18, 2020

The cover letter is still sent for multi-patch contributions, and for single-patch ones, it is included between the --- line and the diff stat.

I agree that it is less important for single-patch contributions, but even in those cases, it is a good space to provide ephemeral context (e.g. to mention on what occasion the contributor stumbled over the problem, or that they want to participate in Outreachy etc)

@phil-blain
Copy link
Contributor Author

phil-blain commented Mar 19, 2020

thanks for the review!

the table and the Cc: example seem too small when I look at it on a phone screen

Fixed.

the table entries would read better to me in the 3rd person

Fixed

The <tt> tag is deprecated

Fixed.

The CC: example has tabs which are causing a positioning problem

Fixed

Not sure where else the cover letter is mentioned

I've added a pointer to an explanation in MyFirstContribution

@phil-blain
Copy link
Contributor Author

Also I thought about adding a third column to compare with the mailing list workflow, is that something that you think would fit/ be valuable ?

Most entries in that column would be something like “Via the "What’s cooking" emails”...

@dscho
Copy link
Member

dscho commented Mar 20, 2020

I thought about adding a third column to compare with the mailing list workflow, is that something that you think would fit/ be valuable ?

While that might be interesting to already-active contributors familiar with the mailing list workflow, the main goal of GitGitGadget is to support new contributors (who generally want to tackle challenges in the code, not in the contributing process), therefore I'd like to avoid that column here. Maybe in an FAQ?

@phil-blain
Copy link
Contributor Author

OK I agree. Let's leave it at that for now then.

@dscho dscho merged commit 8d26c4b into gitgitgadget:master Mar 20, 2020
@dscho
Copy link
Member

dscho commented Mar 20, 2020

Thank you very much!

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.

3 participants