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

Ajout palettes couleur #11

Merged
merged 10 commits into from Apr 27, 2020
Merged

Ajout palettes couleur #11

merged 10 commits into from Apr 27, 2020

Conversation

tvroylandt
Copy link
Contributor

J'ai pris les palettes existantes et j'ai ajouté des palettes à ma sauce en les combinant.

Il faudra certainement revoir les combinaisons et ajouter une vignette pour présenter les palettes + l'utilisation.

@tvroylandt
Copy link
Contributor Author

tvroylandt commented Apr 27, 2020

En oubliant pas le NAMESPACE c'est mieux... J'annule et je refais.

J'oublie toujours des bouts à droite à gauche...
Ca viendra avec l'expérience (ou pas), mais vive GH Actions !

@tvroylandt tvroylandt linked an issue Apr 27, 2020 that may be closed by this pull request
R/colors.R Outdated Show resolved Hide resolved
R/colors.R Outdated Show resolved Hide resolved
R/colors.R Show resolved Hide resolved
@RLesur RLesur self-requested a review April 27, 2020 16:37
Copy link
Member

@RLesur RLesur left a comment

Choose a reason for hiding this comment

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

J'ai regardé le rapport du check et j'ai vu qu'il y avait des warnings.
J'ai réalisé que j'avais paramétré GH Actions de façon trop "laxiste".
J'ai donc modifié cela dans 81388f0

Il reste donc quelques warnings et notes à résoudre avant de pouvoir fusionner.

@tvroylandt
Copy link
Contributor Author

C'est une bonne idée. Merci Romain

Merge branch 'master' into colors

# Conflicts:
#	.Rbuildignore
@codecov-io
Copy link

Codecov Report

Merging #11 into master will decrease coverage by 2.68%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #11      +/-   ##
=========================================
- Coverage    4.00%   1.31%   -2.69%     
=========================================
  Files           2       3       +1     
  Lines          25      76      +51     
=========================================
  Hits            1       1              
- Misses         24      75      +51     
Impacted Files Coverage Δ
R/colors.R 0.00% <0.00%> (ø)

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 a233829...b3d97a7. Read the comment docs.

@MaelTheuliere MaelTheuliere merged commit 91b6df6 into master Apr 27, 2020
@RLesur
Copy link
Member

RLesur commented Apr 27, 2020

@MaelTheuliere pour info, au moment où tu mergeais, j'étais en train de reprendre la documentation de cette PR (je ne faisais que de la mise en forme), j'aurais dû prévenir, désolé. J'ai donc poussé directement sur master : 19d7e1e

La branche ggplot2 a donc un commit de retard.

@RLesur RLesur deleted the colors branch April 27, 2020 21:03
RLesur added a commit that referenced this pull request Apr 27, 2020
@MaelTheuliere
Copy link
Collaborator

Désolé @RLesur j'avais mal interprété le "approved", pour moi ça voulait dire que vous attendiez plus qu'un ok de moi sur la pr

@tvroylandt
Copy link
Contributor Author

Merci d'avoir mergé.
Ca m'intéresse d'avoir un peu vos attendus sur les PR, j'ai pas l'habitude.

si je résume :

  • check ok avec 0 / 0 / 0 ;
  • usethis::use_tidy_description()
  • NEWS à jour
  • ... ?

@RLesur
Copy link
Member

RLesur commented Apr 28, 2020

Je ne sais pas pour @MaelTheuliere, mais personnellement j'essaie d'avoir dans les PR :

  • check 0/0/0 (le CI attend maintenant 0/0/x mais je peux le paramétrer pour qu'il n'accepte que 0/0/0)
  • NEWS à jour
  • lier les PR aux issues correspondantes

Bon, après il m'arrive d'oublier un de ces trucs...

Ensuite, en cadeaux bonus mais sans se prendre la tête si ce n'est pas fait :

  • usethis::use_tidy_description()
  • pas trop dégrader le test coverage (c'est pas la mort non plus s'il baisse)
  • juste pour vérifier, un petit coup de temps en temps de lintr::lint_package() (ça ne fait pas de mal)

Enfin, au niveau du merge des PR, j'ai une grosse préférence pour :

  • squash commits
  • supprimer les branches

@tvroylandt
Copy link
Contributor Author

Super merci !

Je viens de découvrir le squash commits.

J'ai aussi vu que tu avais rajouté de la doc et notamment les histoires d' inheritsParams et tout que je connaissais pas trop et qui sont intéressantes. Merci !

@RLesur
Copy link
Member

RLesur commented Apr 28, 2020

@tvroylandt oui, je me suis permis de le faire directement : j'étais à peu près certain que ça t'irait.

@MaelTheuliere
Copy link
Collaborator

MaelTheuliere commented Apr 28, 2020

moi j'ai l'habitude du check, de lier la PR aux issues, de styler, de supprimer les branches. Je pratiquais pas la mise à jour en continue de la news et le test coverage. mais cette pratique me va bien, c'est l’occasion d'apprendre

@RLesur
Copy link
Member

RLesur commented Apr 28, 2020

Ah et vu qu'on avance bien, il va falloir qu'on commence à incrémenter les versions en augmentant le numéro de patch.

@cdugeai
Copy link
Collaborator

cdugeai commented Jan 5, 2023

Hello,
Je me demandais d'où provenaient les couleurs de la palette ? Directement de la marque de l'état, avec certains choix ?

@RLesur
Copy link
Member

RLesur commented Jan 6, 2023

De mémoire, on était parti de la marque Etat, oui.

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.

Echelle de couleurs
5 participants