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

Jazzband branding #106

Merged
merged 5 commits into from
Jun 21, 2020
Merged

Jazzband branding #106

merged 5 commits into from
Jun 21, 2020

Conversation

Bouke
Copy link
Collaborator

@Bouke Bouke commented Jan 24, 2020

Related to #105.

@codecov
Copy link

codecov bot commented Jan 24, 2020

Codecov Report

Merging #106 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #106   +/-   ##
=======================================
  Coverage   88.92%   88.92%           
=======================================
  Files          10       10           
  Lines         280      280           
  Branches       31       31           
=======================================
  Hits          249      249           
  Misses         23       23           
  Partials        8        8           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0c5337b...1ec7a6f. Read the comment docs.

@Bouke
Copy link
Collaborator Author

Bouke commented Jan 24, 2020

@ricco386 could you review this PR?

Copy link

@ricco386 ricco386 left a comment

Choose a reason for hiding this comment

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

@Bouke all major branding changes looks good, but I have one suggestion at the end:

  • Fix all links in the docs (and README file etc) from old to new repo
  • Add the Jazzband badge to the README file
  • Add the Jazzband contributing guideline to the CONTRIBUTING.md or CONTRIBUTING.rst file

I have noticed that you have removed CONTRIBUTORS.rst, I think it is nice to give contribution to the people who have helped with the project. Did you do that so it is not confused with newly added CONTRIBUTING.rst file?

If I may suggest: I would leave a thanks to all contributors note with a link at the bottom of the README, to contributors page: https://github.com/jazzband/django-user-sessions/graphs/contributors and this way it will always reflect the reality automatically. I am not entirely sure if there is something similar on Transifex?

@Bouke
Copy link
Collaborator Author

Bouke commented Jan 31, 2020

Thank you for the review; I've just pushed a small fix.

Did you do that so it is not confused with newly added CONTRIBUTING.rst file?

I did it because the file didn't get updated in 5 years; so neither code nor translation contributor of the past 5 years did see the need to add their names to the list. So instead of keeping this artifact of the past, I would invite people to look at the git authors, by clicking the newly added "contributors" link in the readme.

I am not entirely sure if there is something similar on Transifex?

I just checked, and there is no such thing. Teams are not exposed publicly, and even for an admin you'd have to go into each separate language to see the members. Maybe we should even ditch Transifex and just manage the translations in code instead. But let's leave that discussion for #105.

Copy link

@ricco386 ricco386 left a comment

Choose a reason for hiding this comment

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

Looks good, we can merge thins PR.

Thank you for the update. 👍

@Bouke
Copy link
Collaborator Author

Bouke commented Jan 31, 2020

I cannot merge the PR as some checks are failing. As I'm no longer listed as admin of this project, I cannot drop those failing checks. Dunno what to do here?

@Bouke Bouke merged commit 351389e into master Jun 21, 2020
@Bouke Bouke deleted the jazzband branch June 21, 2020 18:10
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