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

Add more examples to playground #67

Merged
merged 23 commits into from
Jan 20, 2022
Merged

Add more examples to playground #67

merged 23 commits into from
Jan 20, 2022

Conversation

katauber
Copy link
Member

@katauber katauber commented Dec 9, 2021

@TobiasNx TobiasNx requested a review from fsteeg January 11, 2022 15:34
Copy link
Member

@fsteeg fsteeg left a comment

Choose a reason for hiding this comment

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

Here are some things I noticed (this is actually a functional review):

  • The dropdown should have some visual indication that unlike the other buttons, it's a dropdown (e.g. a triangle/caret pointing downwards)
  • The currently selected sample name should be visible, e.g. in the dropdown (both this and the first point should come for free with a UI component using/corresponding to the <select> HTML tag)
  • From a usage standpoint, it would be really nice to see the input data when using a URL, e.g. if the data editor would be filled by that data (but still remained grayed out)
  • This issue is emphasized by the fact that currently, 6 of 8 examples read from a URL. What I imagined was to have the examples from the initial comment with Playground input data, to see input, transformation and output for these different workflows (except probably the OAI-PMH workflow, though an XML-input example would be nice too).
  • The Web_Pica_to_MarcXML sample produces no output for me (this branch depends on metafacture-core 5.3.1); also the flux seems pretty complex for an example (Pica-to-Marc21-toMarcXml?)
  • I think we discussed (but didn't write down?) the idea to include numbering in the example file names to allow a tutorial-like ordering of the examples

Not sure what to approach here and what to do in separate issues. I think our process was a bit off here, maybe because @TobiasNx was both functional reviewer (for the integration of examples in the Playground) and implementor (for the examples themselves), so there was no functional review for the examples. Or it's just a bad idea to do multiple examples at once. Maybe we should have, and should in the future, open individual issues for each individual example.

To move forward and avoid keeping this branch longer running, we could:

  • Remove all but two examples with URL input here (e.g. keep OAI-PMH_MODS_to_JSON and Web_MarcXML-to-JSON, which were mentioned in the initial comment), merge and deploy this
  • Open new issues for dropdown UI, display URL input, HTML-to-JSON example, each of the removed examples, and numbering

I've moved these workflows to a disabled/ subfolder and made a small tweak to allow folders under examples/ in 38c1eeb. We could then move them back when working on the new issues.

@dr0i
Copy link
Member

dr0i commented Jan 13, 2022

The Web_Pica_to_MarcXML sample produces no output for me (this branch depends on metafacture-core 5.3.1)

Yes, this was fixed with metafacture/metafacture-core#435. Would it make sense to use metafacture-core version master-SNAPSHOT ? This implies to build metafacture-core locally, see #25 (comment). Did this for https://test.metafacture.org/playground/.

@katauber
Copy link
Member Author

katauber commented Jan 18, 2022

  • The dropdown should have some visual indication that unlike the other buttons, it's a dropdown (e.g. a triangle/caret pointing downwards)
  • The currently selected sample name should be visible, e.g. in the dropdown (both this and the first point should come for free with a UI component using/corresponding to the <select> HTML tag)

I deployed these changes to https://test.metafacture.org/playground/. @TobiasNx should re-do functional review because the dropdown menu changed. Please re-assign @fsteeg for code review afterwards.

  • I think we discussed (but didn't write down?) the idea to include numbering in the example file names to allow a tutorial-like ordering of the examples

I remember later on we decided not to use numbers. We should discuss this again, if we want an issue for numbering the examples.

  • Open new issues for dropdown UI, display URL input, HTML-to-JSON example, each of the removed examples, and numbering

Opened:

I did not openend an issue for each of the removed examples. Could please someone else (@fsteeg or @TobiasNx?) open the issues for these examples.

@katauber katauber removed their assignment Jan 18, 2022
@katauber katauber merged commit d100053 into main Jan 20, 2022
@katauber katauber deleted the 25-examples branch January 20, 2022 07:56
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.

Shared workflows have disabled Data and Fix/Flux editors Collect & add more Playground examples
4 participants