galaxyproject / galaxy

Data intensive science for everyone.
https://galaxyproject.org
Other
1.38k stars 992 forks source link

Encourage documenting features as part of a PR contribution #2829

Closed afgane closed 5 years ago

afgane commented 8 years ago

Stemming from a discussion in prep for a release (https://github.com/galaxyproject/galaxy/pull/2821#issuecomment-241434959), I'd like to start a discussion about strongly encouraging contributors to add documentation about features they PR. At a minimum, this encouragement would be just a note in CONTRIBUTING.md file. The other extreme would be a requirement to point to user and/or admin facing docs on the wiki prior to a PR merge. My view is that without proper documentation, many features just see adoption by people waiting for the given feature rather than being discovered by others as well.

The current model of listing features in the release notes is great but those are temporal and once a release is a bit dated, users do not visit those notes much. Also, those notes are not grouped by functionality as proper docs are likely to be. They also tend to be developer centric.

Thoughts on the happy middle ground?

jmchilton commented 8 years ago

In the past I have tried to group related items together in the release notes, I'd encourage anyone who notices items that should be grouped closer together to open PRs or write comments that indicate these things should be grouped closer. I also have traditionally reworded developer-facing PR titles into user-facing or deployer-facing wording - again if you find particular items that are this way in the release notes I'd encourage you to reword them. The release notes should target many audiences - not just developers.

hexylena commented 8 years ago

The other extreme would be a requirement to point to user and/or admin facing docs on the wiki prior to a PR merge

I think it might not be feasible to do this (even though I like the idea on the surface). Too large a number of PRs are so small or not-interesting from an end-user/admin-user perspective that it doesn't make sense to require it for all.

Possible criteria:

Might be nice to expand this into a flow chart

dannon commented 8 years ago

I'd assume this, as it stands in theory, would only refer to PRs tagged enhancement/feature, right?

Anything else is going to get out of hand very quickly. And we already have a problem with wiki pages being write-once, read-seldom, rewrite-never.

dannon commented 8 years ago

(sorry, that damn 'close and comment' button)

hexylena commented 8 years ago

@dannon what about

Label Needs Docs
kind/bug maybe
kind/enhancement yes
kind/feature yes
kind/refactoring no
kind/testing no
afgane commented 8 years ago

Personally, I think @erasche's thinking sounds very reasonable. Also, docs associated with a PR don't necessarily need a dedicated page but perhaps a paragraph or similar on an existing page - just something that describes/explains how to use a feature, whether it be by admins or end-users.

dannon commented 8 years ago

@erasche Just perusing https://github.com/galaxyproject/galaxy/issues?q=label%3Akind%2Fenhancement makes me think 'enhancement' should be a maybe at most. And 'bug', I'd say no.

afgane commented 8 years ago

The enhancement should probably be a docs edit if needed, right? That would also help with keeping more docs up to date vs. writing things once and never/rarely updating.

dannon commented 8 years ago

I guess my ideal stance-o-the-project here would be for committers to recognize the fact that to use features, users need instructions, and to ask for them when necessary at merge.

dannon commented 8 years ago

@afgane If you look through the 'enhancements', no, that is not the case.

(not commonly the case, looking through the list of how we currently use the tag, I should have said -- of course there are enhancements where the documentation might need an update)

jxtx commented 8 years ago

On a quick glance it looks like PRs tagged with enhancement that might not need docs are probably mistagged refactorings. Anything that changes any aspect of how Galaxy looks, functions, is configured, etc probably needs docs.

I agree roughly with @erasche's table, but I also think this is hard to legislate. It will always be subjective, but I think we should err on the side of more docs. We are far from there.

dannon commented 8 years ago

@jxtx Totally agree both that we should err on the side of more docs, and to the sentiment that it will always be subjective.

So do we all just agree to, when something comes in that seems like it should be documented, updated, etc., -- look for said documentation and simply have the conversation about it at PR merge time?

hexylena commented 8 years ago

labels Yes, I should amend the table, "yes" = "yes and these other questions on a flowchart too." There is definitely a decision point to be made with respect to which ones need docs

I think it would help if we make an effort to go through a page of the maybes and see if we can have some criteria for doing this, so we can have some consistent rationale for what needs docs and what doesn't. By we, I of course mean I've done this for kind/enhancement just as an experiment (this feels like an ML problem anyway, we have issues with a set of tags, do those tags signal that they should have docs). Here is a table and my 2 cents from the kind/enhancement tag.

PR Title Tags (Minus status/*, kind/enhancement) Needs docs Actionable Information Guiding Decision
Add "Data Export" category to MTS and TTS area/toolshed No Only affects TSs
Add column names to BED datatype so that column names are displayed i… area/datatypes No (Unless we start documenting filetypes somewhere) Datatypes are not documented in the docs (except auto-generated. Is that enough? Do we need more user-facing ones?)
Add implicit conversions target extension to hda name when listed as selection option area/UI-UX No Too subtle a UI change (dislike this rationale)
Add makefile targets for running client-build watch modes (none) ? no tags that are useful (i.e. should be retagged).
Allow repo type change through TS API area/API area/toolshed Yes Affects end users/admins
Avoid user role checks for individual dataset when building the tool form models area/performance area/tool-framework No Subtle UI/UX
Charts revision, part 2 area/UI-UX area/visualizations Yes Visible to end users
Cleanup of and performance enhancements to the API installation of repositories. area/API area/performance area/toolshed kind/feature kind/refactoring No, area/performance
Collection Operator tool : Merge Collection area/dataset-collections kind/feature yes End user visible
Improve startup scripts area/framework kind/bug Yes Admin visible
List resolver toolshed packages area/API area/UI-UX kind/bug yes Admin visible
More configurability for containerized jobs area/jobs Yes Admin visible
Properly refresh history contents after submitting workflow through api scheduler area/UI-UX Maybe? Release notes enough, or ..?
Remove enable_beta_tool_command_isolation config option area/documentation area/jobs Yes (with a note to remove the docs in N releases?) Affects galaxy.ini
Revise repeat block view, remove unecessary table wrapper area/UI-UX No Subtle UI/UX
Slightly revise auto configuration of loggers area/toolshed No "slightly revise"
Update python-ldap module area/framework No Dependency updates are not interesting
Correct license name in nodejs proxy area/GIEs minor No tagged minor
HDT format added and turtle sniffer improved area/datatypes Yes (Maybe we should start documenting datatypes) Definitely in release notes
Regularize galaxy.ini.sample. area/documentation No ini-only change, therefore not interesting to admin (if there were code changes, then maybe new parameter added)
[16.07] curated release notes for 16.07 area/documentation No backport
[16.04] Backport #2734 area/UI-UX No backport
[16.07] Add dependency_resolvers_conf.xml.sample to common_startup.sh area/framework No backport
[16.07] Small followup for #2759 area/documentation kind/testing minor No backport
[16.07] install Conda at startup; enable conda_auto_init in sample conf area/admin kind/refactoring No backport
dannon commented 8 years ago

@erasche So the short answer is that it's very subjective and we should play it by ear? 15/25 "no docs", just on this page.

hexylena commented 8 years ago

The conclusion from that was that there is a pretty easy-to-see checklist, but we will have to do this manually or tag our issues better:

martenson commented 5 years ago

I hope that with the new user-centric release notes we have remedied a chunk of this issue.

Please feel free to reopen/comment if there is more to discuss at any future point.