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

[24.2] Fix some collection builder help and language choices. #19244

Conversation

jmchilton
Copy link
Member

The list help instructions are just wrong and didn't dispatch on the type of components being rendered and even for the right modality was using old names of buttons. That part should be an uncontroversial bug fix. The other commit is language choices - I consider a lot of the old language both too pedantic and then overtly incorrect in places despite that - I prefer all these language choices here more. Incorrect language here and in training documentation and tools has resulted in collections seeming a lot more complex and a lot less beautiful than they are - and I should have worked harder to review everything as it came in.

How to test the changes?

(Select all options that apply)

  • This is a refactoring of components with existing test coverage.

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

@ahmedhamidawan
Copy link
Member

Thank you so much for these changes! I totally forgot to update the help text for each builder; I had even added a TODO comment... 🤦‍♂️
Please let me know if I can help out!

@jmchilton jmchilton marked this pull request as ready for review December 4, 2024 14:36
@github-actions github-actions bot added this to the 25.0 milestone Dec 4, 2024
"Collections of paired datasets are ordered lists of dataset pairs (often forward and reverse reads). ",
"These collections can be passed to tools and workflows in order to have analyses done on each member of ",
"the entire group. This interface allows you to create a collection, choose which datasets are paired, ",
"Lists of pairs are an ordered list of individual dataset paired together in their own collection (often forward and reverse reads). ",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"Lists of pairs are an ordered list of individual dataset paired together in their own collection (often forward and reverse reads). ",
"List of pairs is an ordered list of individual datasets paired together in their own collection (often forward and reverse reads). ",

Copy link
Member

Choose a reason for hiding this comment

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

using singular sounds clearer to me here, but feel free to disregard

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 think I could agree if it was "A list of pairs" and below "A list" (instead of "This list"). To me "List of pairs is" feels odd. I'll commit that change - it stills satisfies your point that singular would sound better - and I think agree with that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ahh... reading it in the broader context I think we are describing lists generally in that first sentence. What if we kept the plural here but we added a sentence at the beginning like "This interface allows you to build a new Galaxy list of pairs. List of pairs are an ordered....". If that still reads odd to you - I'll try something else - I really appreciate your perspective here and for you taking the time to comment. I want this language to read well and feel universal. The language around collections has repeatedly escaped me in ways I'm uncomfortable with.

Copy link
Member

Choose a reason for hiding this comment

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

I like the added sentence. It is hard (and important imho) to distinguish where we are describing a concrete interface user is looking at and where we explain broader concepts.

"These collections can be passed to tools and workflows in order to have analyses done on each member of ",
"the entire group. This interface allows you to create a collection, choose which datasets are paired, ",
"Lists of pairs are an ordered list of individual dataset paired together in their own collection (often forward and reverse reads). ",
"These lists can be passed to tools and workflows in order to have analyses done on each member of ",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"These lists can be passed to tools and workflows in order to have analyses done on each member of ",
"This list can be passed to tools and workflows in order to have analyses done on each member of ",

@jmchilton jmchilton added the release-testing-24.2 Issues stemming from 24.2 release testing process and PRs to address them label Dec 5, 2024
@jmchilton jmchilton force-pushed the collection_builder_language branch 2 times, most recently from 54a56cd to f248cb6 Compare December 6, 2024 14:54
jmchilton and others added 4 commits December 6, 2024 10:42
This was describing the "selection" mode instructions for the free build from history version - if that makes any sense.
@jmchilton jmchilton force-pushed the collection_builder_language branch from f248cb6 to 83c19e0 Compare December 6, 2024 15:42
Copy link
Member

@ahmedhamidawan ahmedhamidawan left a comment

Choose a reason for hiding this comment

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

Excellent changes, thank you!

@ahmedhamidawan ahmedhamidawan merged commit 73ce5b8 into galaxyproject:release_24.2 Dec 6, 2024
25 of 27 checks passed
@martenson martenson deleted the collection_builder_language branch December 8, 2024 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/UI-UX kind/bug release-testing-24.2 Issues stemming from 24.2 release testing process and PRs to address them
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants