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

Work on use_github_labels() #249

Merged
merged 9 commits into from
Feb 14, 2018
Merged

Work on use_github_labels() #249

merged 9 commits into from
Feb 14, 2018

Conversation

jennybc
Copy link
Member

@jennybc jennybc commented Feb 14, 2018

  • Export gh_labels() tidy_labels() which returns named vector of in-house standard labels and colours
  • Add labels argument, in case anyone wants something other than gh_labels() tidy_labels()
  • Fix buglet that prevented modification of exactly 1 colour + handle a length-0 edge case
  • Add "good first issue" and "help wanted", with GitHub default colours (now tweaked)
  • Adopt GitHub default colours for "bug" (just a different red) and "feature" (the light blue they use for "enhancement")
  • Beefed up the docs
  • Styled with styler

This repo has had this version of use_github_labels() called on it, so here's the effect:
https://github.com/r-lib/usethis/labels

Closes #168

More about color changes inspired by the official GitHub colors:

  * Switched to their red for "bug"
  * Our "feature" now uses GitHub's color for "enhancement"

https://twitter.com/github/status/954467266434945024

"Parts of GitHub might look a little different today. We've changed the text color for issue labels to meet accessibility standards based on WCAG guidelines."
@jennybc jennybc requested a review from hadley February 14, 2018 07:16
#'
#' @param delete_default If `TRUE`, will remove default labels.
#' `gh_labels()` returns the labels and colours commonly used by tidyverse
Copy link
Member

Choose a reason for hiding this comment

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

Should this include tidy since it's a tidyverse convention?

Copy link
Member Author

Choose a reason for hiding this comment

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

gh_labels() --> tidy_labels() in 7d7a544

#' It does not generally remove labels, unless you explicit ask to
#' remove GitHub's defaults.
#' @description `use_github_labels()` creates new labels and/or changes label
#' colours. It does not generally remove labels, unless you explicitly ask to
Copy link
Member

Choose a reason for hiding this comment

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

unless explicitly requested with remove = TRUE?

Copy link
Member Author

Choose a reason for hiding this comment

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

Reworded to be mention delete_default = TRUE

#' use_github_labels(delete_default = TRUE)
#' }
use_github_labels <- function(labels = gh_labels(),
delete_default = FALSE,
Copy link
Member

Choose a reason for hiding this comment

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

Should this be remove = FALSE?

Copy link
Member Author

Choose a reason for hiding this comment

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

Are you suggesting that remove = TRUE requests to drop labels that are not present in labels, regardless of whether they are GitHub defaults?

Based on my interactions with usethis labels while working on this, that would create more work. For example, pretty sure I would have accidentally deleted the non-package label by now.

def_labels <- setdiff(cur_labels[default], names(gh_labels))
if (delete_default && length(cur_labels) > 0) {
default <- vapply(cur_labels, "[[", "default", FUN.VALUE = logical(1))
def_labels <- setdiff(cur_label_names[default], names(labels))

if (length(def_labels) > 0) {
done("Removing default labels: ", collapse(value(def_labels)))
Copy link
Member

Choose a reason for hiding this comment

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

default -> other ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Now reads "Removing labels", which should stand the test of time.

)
#' @rdname use_github_labels
#' @export
gh_labels <- function() {
Copy link
Member

Choose a reason for hiding this comment

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

I don't love the current colours, but I find the new colours ugly 😞

Copy link
Member Author

Choose a reason for hiding this comment

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

I reverted the changes to existing colours.

@hadley
Copy link
Member

hadley commented Feb 14, 2018

Would the docs for this function be a good place to park out descriptions of when to use the different labels?

@jennybc jennybc mentioned this pull request Feb 14, 2018
@jennybc
Copy link
Member Author

jennybc commented Feb 14, 2018

Would the docs for this function be a good place to park out descriptions of when to use the different labels?

Yes, I opened #252 for that. We could merge this in the meantime.

I reverted the color changes and picked a nicer violet for "good first issue".

This repo has had the proposed version of use_github_labels() called on it, so here's the effect:
https://github.com/r-lib/usethis/labels

Can I merge? It's easy to tweak colours in future.

@jennybc jennybc merged commit 2d4479b into master Feb 14, 2018
@jennybc jennybc deleted the github-issue-labels branch February 14, 2018 21:25
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.

None yet

2 participants