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

Touch up issue labels. #4912

Merged
merged 2 commits into from Nov 2, 2017

Conversation

Projects
None yet
5 participants
@jmchilton
Member

jmchilton commented Nov 1, 2017

  • Add some labels that have been added to Github but not this list (area/tool-dependencies, area/i18n, area/security).
  • Add subworkflows which was added to Github as area/subworkflows but which I think should be area/workflows/subworkflows. Objections?
  • Add various specific testing areas (integration, selenium, and api) and a general testing area for bugs and enhancements for other parts of testing.*
  • Add labels I think should be there but aren't (area/cwl, area/objectstore, area/upload, area/webhooks).

* I know there is a kind/testing, but I really dislike using it. I initially thought it should be an area and I still do. I'm either making enhancements or bug fixes to testing - I think kind/bug, kind/enhancement, and kind/refactoring apply to the area of testing. Maybe the new more specific areas for testing make that clear? I wanted to see all the recent Selenium changes but I didn't have a real clear way to do that without this. Maybe a compromise if others haven't come around to my way of seeing things is to keep the area sub-types of testing (e.g. area/testing/selenium) but drop area/testing from this list?

Touch up issue labels.
- Add some labels that have been added to Github but not this list (tool-dependencies, i18n, security).
- Add subworkflows which was added to Github as ``area/subworkflows`` but which I think should be ``area/workflows/subworkflows``.
- Add various specific testing areas (integration, selenium, and api) and a general testing area for bugs and enhancements for other parts of testing.*
- Add labels I think should be there but aren't (cwl, objectstore, upload, webhooks)

* I know there is a kind/testing, but I initially thought it should be an area and I still do. I'm either making enhancements or bug fixes to testing - I think ``kind/bug``, ``kind/enhancement``, and ``kind/refactoring`` apply to the ``area`` of testing. Maybe the new more specific areas for testing make that clear? I wanted to see all the recent Selenium changes but I didn't have a real clear way to do that without this. Maybe a compromise if others haven't come around to my way of seeing things is to keep the area sub-types of testing (e.g. ``area/testing/selenium``) but drop ``area/testing`` from this list.
@martenson

This comment has been minimized.

Member

martenson commented Nov 1, 2017

I tested the GH UI and it seems that even labels this long are still rendered well.

I am indifferent on 3rd level labels (area/workflows/subworkflows) but overall 👍 for additional ones.

@@ -124,17 +125,28 @@ particular domain, as well as more organized release notes.
- ``area/framework``
- ``area/GIEs``
- ``area/histories``
- ``area/i18n``
- ``area/jobs``

This comment has been minimized.

@nsoranzo

nsoranzo Nov 1, 2017

Member

What about area/libraries?

This comment has been minimized.

@nsoranzo

nsoranzo Nov 2, 2017

Member

And I mean data libraries, not sure it was clear.

@nsoranzo

This comment has been minimized.

Member

nsoranzo commented Nov 1, 2017

I know there is a kind/testing, but I really dislike using it. I initially thought it should be an area and I still do.

💯 Same here, I objected when the labels where created and still would like to have area/testing

@dannon

This comment has been minimized.

Member

dannon commented Nov 1, 2017

@nsoranzo If I was the one that objected, consider it rescinded. I don't really care strongly about these labels anymore. For me, they just have not proven useful at all, outside of status/{review|WIP} and triage.

@nsoranzo

This comment has been minimized.

Member

nsoranzo commented Nov 1, 2017

I think kind/* (except for kind/testing) are useful for 1) release notes and 2) when looking at open PRs at release freeze meetings.
But we may drop area labels if they're not of use.

@dannon

This comment has been minimized.

Member

dannon commented Nov 1, 2017

@nsoranzo Yeah, you're right about kind/* being useful, I should have mentioned those -- didn't occur to me when I was typing my off-the-cuff comment since the uses are intermittent, but when you need those labels, they're necessary for sure. I'd be in favor of dropping area completely.

@galaxybot galaxybot added this to the 18.01 milestone Nov 1, 2017

@martenson

This comment has been minimized.

Member

martenson commented Nov 1, 2017

I use area for issue/pr filtering almost every day.

@dannon

This comment has been minimized.

Member

dannon commented Nov 2, 2017

@martenson Well, that's good to know. If most people find the area labels genuinely useful, I'm of course in favor of keeping them; was just primarily conveying to @nsoranzo that I wouldn't argue with any changes to area/testing or whatever because I don't personally have an opinion anymore about their usage, since I don't use them.

When I wrote all this up they seemed like a great idea to me, but in actual day-to-day use they haven't helped me at all. I read all the PRs as they come in regardless of area and the volume is low enough that they're not useful for filtering (for me). In my day-to-day wrangling of issues I tend to dig through the backlog with keyword search, not using labels or browsing. Again, just my experience, and if folks find them valuable I'm fine with keeping it around.

@jmchilton

This comment has been minimized.

Member

jmchilton commented Nov 2, 2017

Agreed generally that kind and status are more useful day-to-day than area. I don't use area filtering ever for PRs - but I do use it for issues - I've filtered collections and workflows at least and just gone through them all. But I also go through every PR - so when I'm search for a PR I know what I'm searching for, if I was an admin or less connected to the project I think I'd be more likely be going through release PRs and filtering on area. @dannon started +1 on area/ tags and has drifted toward 0 - I'm the inverse - I started -.5 on them and have drifted toward 0 - only because tagging isn't nearly as onerous as I feared it would be. In retrospect the process seems pretty good to me. I also think the next time I take a pass at automatic release note generation I will group similar PRs together based on area, I think that would both make better release notes and ease the manual portion afterward. (I kind of think including the area in the release notes would be useful also - but it might be a bit much - probably off topic for this PR.)

I've included area/libraries and dropped kind/testing based on comments here.

area/libraries reminded me that I've wanted that in the past but also I've wanted a tag for Galaxy's dependencies before - so I've added area/dependencies also.

Thanks for the conversation all - happy to keep revising based on discussion.

@martenson martenson merged commit 21c09f7 into galaxyproject:dev Nov 2, 2017

6 checks passed

api test Build finished. 306 tests run, 4 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished. 162 tests run, 0 skipped, 0 failed.
Details
integration test Build finished. 57 tests run, 0 skipped, 0 failed.
Details
lgtm analysis: JavaScript No alert changes
Details
toolshed test Build finished. 577 tests run, 0 skipped, 0 failed.
Details
@martenson

This comment has been minimized.

Member

martenson commented Nov 2, 2017

I syncd' the labels in GH

@jmchilton

This comment has been minimized.

Member

jmchilton commented Nov 2, 2017

Awesome - thanks for the merge and the sync @martenson.

@jmchilton jmchilton deleted the jmchilton:label_touch branch Nov 2, 2017

@martenson

This comment has been minimized.

Member

martenson commented Nov 2, 2017

I see the labels as mostly not for us (who read every PR), but for the community. Also this is a good idea:

group similar PRs together based on area

@martenson

This comment has been minimized.

Member

martenson commented Nov 2, 2017

(kind/testing has been renamed to area/testing - so the instances are preserved)

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