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

New Tutorial: Building and Manipulating Collections with Rules #676

Merged
merged 3 commits into from
May 25, 2018

Conversation

jmchilton
Copy link
Member

@jmchilton jmchilton commented Jan 29, 2018

@hexylena
Copy link
Member

@jmchilton working on this today to make it more like the other trainings. I'll open a PR in a bit. Is there anything other than stylistic stuff that makes this WIP?

@jmchilton
Copy link
Member Author

working on this today to make it more like the other trainings

Sounds great, I was unsure if this is ultimately the correct home for this content since it is more technical manual for a core piece of the UI but if you can make it feel more like the rest that is probably great.

Is there anything other than stylistic stuff that makes this WIP?

The examples are what I want them to be, I'm not confident in the copy editing - spelling, grammar, length of supporting text in general, etc.. I also want to regenerate the screenshots with the final 18.05 version but if you don't see any glaring problems I'll skip that. If you do think the screenshots are quite out of date, I can regenerate them. The screenshots are generated from test cases in the code itself so it is really easy to modify them if there is anything you'd like to see different in them. This is a lot of binary content so I would probably rebase the images into the original commit.

@jmchilton jmchilton changed the title [WIP] New Tutorial: Building and Manipulating Collections with Rules New Tutorial: Building and Manipulating Collections with Rules May 25, 2018
@hexylena
Copy link
Member

The examples are what I want them to be, I'm not confident in the copy editing - spelling, grammar, length of supporting text in general, etc..

great, I can help with this :)

I also want to regenerate the screenshots with the final 18.05 version but if you don't see any glaring problems I'll skip that.

Skip that for now, the ability to regenerate them automatically is really cool, but I think I'd like to trim down a few of them to make their scope more focused. (I know, sub-optimal, and now we can't regenerate those automatically, but not sure I'm up to defining some way to automatically re-crop the images.)

Just a note, $& is used in a lot of examples but this seems to have been changed to \0? Does \1, \2,... work for matching groups now? If so I'll document that in a tip box.

@jmchilton
Copy link
Member Author

Skip that for now, the ability to regenerate them automatically is really cool, but I think I'd like to trim down a few of them to make their scope more focused.

This sounds like a mistake IMO, I really wanted living documentation. It is better to have images that are too large than images that 4 years out of date IMO. No one is ever going to take the time to regenerate all these images manually and this is just the first release of this component.

Just a note, $& is used in a lot of examples but this seems to have been changed to \0? Does \1, \2,... work for matching groups now?

Yeah - we switched from JS to Python syntax for regex. We definitely need to regenerate these images.

@hexylena
Copy link
Member

This sounds like a mistake IMO, I really wanted living documentation. It is better to have images that are too large than images that 4 years out of date IMO. No one is ever going to take the time to regenerate all these images manually and this is just the first release of this component.

completely fair point. If you update the images I'll add an imagemagick script to auto-crop them.

@jmchilton
Copy link
Member Author

completely fair point. If you update the images I'll add an imagemagick script to auto-crop them.

Thank you for giving this to me. I'm realizing now I'm asking for the community to invest in an experiment that will be rough at first and may not actually pay off. But I have this idea for test-driven documentation - tests that prove the documentation is correct, tests that automate the documentation generation, documentation that enhances the readability of the tests, tours that turn into tests and tests that turn into tours - and documentation generated from both. This may be just a one-off shitty piece of documentation that isn't as good as the rest of the examples in the repository but it might turn into a real model for how "a certain kind" of Galaxy documentation in generated - like a living user manual - regenerated fresh for each release and totally automated - a Makefile invocation in the release checklist.

This is why if y'all decide this doesn't belong here I can find a new home for it. If you do want to continue to put up with the experiment rather than imagemagick, can you tell me what DOM elements you want to capture and I could do this as part of the framework that generates these? Would trimming each example that features the uploader widget down to just the uploader be a good first step or not nearly enough? If you want to just crank something out with imagemagick and not wait on me to add DOM cropping - there is a script in the PR (https://github.com/galaxyproject/training-material/pull/676/files#diff-2903354eee0aa269604ca70f999fbb0e) that can be augmented and I can run it for you if you push changes to it.

@hexylena
Copy link
Member

hexylena commented May 25, 2018

@jmchilton the last row of https://raw.githubusercontent.com/jmchilton/galaxy-examples/master/sra_PRJNA355367/PRJNA355367.csv is an SRA link. is that supposed to be that?

Additionally (using the one directly from your github link)

auswahl_169

Edit: Using a tabular version of the same file works perfectly.

@hexylena
Copy link
Member

Ah maybe that explains the "TODO: This screenshot is off…", since it's clearly using tabs rather than commas.

@hexylena
Copy link
Member

@hexylena
Copy link
Member

hexylena commented May 25, 2018

In https://github.com/jmchilton/training-material/blob/dc34b2168f08ea107e55af1bb8e9020333956f14/topics/introduction/images/rules/rules_apply_rules_example_4_14_apply_rules_filtered_and_nested.png what is the first step? "Add column for identifier0"? I don't see a third column, am I maybe missing something?

Oh: now that I have the tool enabled it is clear that it's default.

@jmchilton
Copy link
Member Author

"Add column for identifier0"? I don't see a third column, am I maybe missing something?

That first column should be pulled in automatically from the original list - it is the top level identifier. After that there should be one column. Adding the regex should add two more columns - so I guess using the grouping form of the regex rule. Then there should be three columns. Went to find a Jenkins screenshot and noticed it isn't running tests starting with u - must have messed up the breaking up of the tests somehow 😢.

Copy link
Member

@bgruening bgruening left a comment

Choose a reason for hiding this comment

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

Some general comments:

  • more icons, these are mostly missing
  • more padding around the images would be nice
  • "Click Apply" is sometime bold, sometimes not

I'm sure @bebatut will find many other things that can be improved, but I'm old and my eyes are not that good anymore - so lets merge this and give people a handle to master this awesome feature.

Thanks a lot @jmchilton and @erasche!

@bgruening bgruening merged commit 30f6ebb into galaxyproject:master May 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants