common-workflow-lab / cwl-ts-auto

Autogenerated TypeScript bindings for CWL
Apache License 2.0
6 stars 4 forks source link

Add generated code for cwl-v1.2.1_proposed #2

Closed ZimmerA closed 2 years ago

ZimmerA commented 2 years ago

This is the first version of cwl-ts-auto generated by the codegen currently under development in https://github.com/common-workflow-language/schema_salad/pull/479 using the cwl-v1.2.1_proposed schema

Any bug reports or improvement suggestions are highly appreciated! Keep in mind that this code currently only has been tested with node.js v17 and probably does not work in the browser yet.

If there are any questions, feel free to ask in here or contact me privately. I will try to upload examples on how to use this library shortly!

TODO to merge this:

mr-c commented 2 years ago

Great to see the progress! Any thoughts on how to wire up the tests to run here?

ZimmerA commented 2 years ago

Great to see the progress! Any thoughts on how to wire up the tests to run here?

Yeah, I was going to copy over the workflow from https://github.com/common-workflow-lab/cwl-ts-auto/pull/1 I just didn't get around to doing it yet 😸

codecov-commenter commented 2 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (main@5abb127). Click here to learn what that means. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main       #2   +/-   ##
=======================================
  Coverage        ?   33.54%           
=======================================
  Files           ?      126           
  Lines           ?     6371           
  Branches        ?     1569           
=======================================
  Hits            ?     2137           
  Misses          ?     3939           
  Partials        ?      295           
Flag Coverage Ξ”
unittests 33.54% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more Ξ” = absolute <relative> (impact), ΓΈ = not affected, ? = missing data Powered by Codecov. Last update 5abb127...8e695bf. Read the comment docs.

ZimmerA commented 2 years ago

Change file naming convention to all lower case\

Aww, that is a bummer. Is that a Typescript thing?

Heh, there isn't really a convention on this in ts besides "keep it consistent". I would personally prefer camelCase.ts filenames, but it's hard to autogenerate for class names like CWLVersion, because you would need to know that CWL is an acronym and the 'V' of version has to be capitalized again. Of course, you could add acronyms like CWL and the name conversion function would check against those, but this would get broken by any new acronyms. If you have any idea, let me know!

mr-c commented 2 years ago

Change file naming convention to all lower case\

Aww, that is a bummer. Is that a Typescript thing?

Heh, there isn't really a convention on this in ts besides "keep it consistent". I would personally prefer camelCase.ts filenames, but it's hard to autogenerate for class names like CWLVersion, because you would need to know that CWL is an acronym and the 'V' of version has to be capitalized again. Of course, you could add acronyms like CWL and the name conversion function would check against those, but this would get broken by any new acronyms. If you have any idea, let me know!

Why not keep the case as-is? If people want another case convention than the one used in their source schema salad documents, they can apply a post-codegen transformation, yes?

ZimmerA commented 2 years ago

Why not keep the case as-is? If people want another case convention than the one used in their source schema salad documents, they can apply a post-codegen transformation, yes?

That would break consistency with the filenames in the util folder. Other than that, I agree.

That aside, this is just the file names, the types and classes itself will keep their original name and will also be imported with the original name. The library consumer will never be concerned with the file names. Example for commandlinetool.ts:

import { CommandLineTool } from 'cwl-ts-auto'

Do you think it would still be better to have the file names as is? I am not attached to the 'all lower case' standard πŸ˜„

Edit: I overread the post-codegen transformation part, that could be a good solution!

mr-c commented 2 years ago

@ZimmerA shall we merge this and cut a 0.1 release?

ZimmerA commented 2 years ago

@ZimmerA shall we merge this and cut a 0.1 release?

Can do, I will prepare the release and then we can merge πŸ‘πŸ»

ZimmerA commented 2 years ago

@mr-c I would say this is ready, what do you think?

mr-c commented 2 years ago

Go for it!

mr-c commented 2 years ago

Maybe mention that this is only for CWL v1.2 syntax, and other documents will need upgrading using https://pypi.org/project/cwl-upgrader/ first?

ZimmerA commented 2 years ago

Done!