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

update json-schema template to new structure #688

Merged
merged 1 commit into from
Jul 24, 2020

Conversation

mashehu
Copy link
Contributor

@mashehu mashehu commented Jul 22, 2020

First try at #687
Not sure if we really need to ditch type:objectfor groups as done in the example, because they still contain properties. This would also mean less changes in the other code (which often identifies a group by type==object). But maybe I misunderstood the changes for the new structure. 🤷‍♂️

@ewels
Copy link
Member

ewels commented Jul 23, 2020

Note to self: diff is mostly whitespace, see diff without whitespace

I don't quite understand your comment - you have ditched the type:object in this PR, right? The definitions entries can still have their own properties..

@mashehu
Copy link
Contributor Author

mashehu commented Jul 23, 2020

no, all elements except for parameters are still all type:object as before, including the topmost element with the definitions. The latter was in the example you posted, so I left it like that.

@ewels
Copy link
Member

ewels commented Jul 23, 2020

Yeah, I think that the topmost element needs to be an object. I hadn't noticed that the type:object lines were still there in this PR. I think you can just delete them..?

Maybe I should back up and give a proper explanation for why the nasty hack was needed. I'll add a comment with this to #687

@ewels
Copy link
Member

ewels commented Jul 24, 2020

I was wrong: #687 (comment)

Good to merge ✅ - will do so into a branch so as not to mess up dev until we're ready.

Will start working on updating the rest of the code and fixing the tests.

@ewels ewels changed the base branch from dev to new-new-schema July 24, 2020 14:09
@ewels ewels merged commit 8a3ec77 into nf-core:new-new-schema Jul 24, 2020
@ewels ewels mentioned this pull request Jul 27, 2020
5 tasks
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.

2 participants