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

Include additional fields to manifest #5314

Merged
merged 4 commits into from
Sep 19, 2024

Conversation

jorgee
Copy link
Contributor

@jorgee jorgee commented Sep 19, 2024

Include new fields in the manifest and update the documentation with the new fields

…anifest

Signed-off-by: jorgee <jorge.ejarque@seqera.io>
Signed-off-by: jorgee <jorge.ejarque@seqera.io>
@jorgee jorgee requested a review from a team as a code owner September 19, 2024 10:33
@jorgee jorgee linked an issue Sep 19, 2024 that may be closed by this pull request
Copy link

netlify bot commented Sep 19, 2024

Deploy Preview for nextflow-docs-staging ready!

Name Link
🔨 Latest commit bcc6524
🔍 Latest deploy log https://app.netlify.com/sites/nextflow-docs-staging/deploys/66ec2aca8b83110008ef3aa5
😎 Deploy Preview https://deploy-preview-5314--nextflow-docs-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

docs/config.md Outdated Show resolved Hide resolved
Co-authored-by: Ben Sherman <bentshermann@gmail.com>
Signed-off-by: Jorge Ejarque <jorgee@users.noreply.github.com>
@pditommaso
Copy link
Member

Looks neat. Thin @ewels was suggesting to user icon instead of iconUrl in a recent comment

Signed-off-by: jorgee <jorge.ejarque@seqera.io>
@jorgee
Copy link
Contributor Author

jorgee commented Sep 19, 2024

Looks neat. Thin @ewels was suggesting to user icon instead of iconUrl in a recent comment

Yes, changed.

@pditommaso
Copy link
Member

Well done! 🎉

@pditommaso pditommaso merged commit 33fab52 into master Sep 19, 2024
23 checks passed
@pditommaso pditommaso deleted the 4034-additional-manifest-config-fields branch September 19, 2024 14:31
@ewels
Copy link
Member

ewels commented Sep 19, 2024

Ooh awesome! Thanks @jorgee!

Looks like author / maintainer fields are just strings here. Could we optionally have these as maps as well with additional subfields such as ORCiD, affiliatiob and email? See #4034 (comment)

'cc @pditommaso @bentsherman

@pditommaso
Copy link
Member

Think we said it would be better to keep it flat

@bentsherman
Copy link
Member

The BCO supports a bunch of contributor metadata as a list of maps, so you can do something like name, email, ORCID

https://wiki.biocomputeobject.org/Provenance-domain#Contributors_%E2%80%9Ccontributors%E2%80%9D

I think we could support this structure directly in the config. The manifest.author could be either a string for comma-separated list, or a list of maps for the complete structure. I think name, affiliation, email, and ORCID should be enough, but always easy to add more fields later.

See also: nextflow-io/nf-prov#10

@bentsherman
Copy link
Member

So the list of maps is mainly to provide a more complete BCO manifest

@jorgee
Copy link
Contributor Author

jorgee commented Sep 20, 2024

In fact, there is no check in the author, so you can put whatever valid expression (String, list, list of maps).

manifest {
     author = [ [ name: 'First Author', email: 'author1@mail.com'], [ name: 'Second Author', email: 'author@mail.com']]
}

The above configuration should work. In the nextflow info, it will print the string serialization of the list of maps.
I think we just need to validate the map fields.

@ewels
Copy link
Member

ewels commented Sep 20, 2024

Nice! Yes some validation would be nice just to provide a base level of standardisation.

Though saying that, it could be nice to allow additional arbitrary keys - eg. GitHub handle or I'm sure there are others.

@ewels
Copy link
Member

ewels commented Sep 20, 2024

Ok so if it already works, shall we say that a short term solution is to just modify the documentation to say that it can be a string or a map? Sounds like that would be sufficient for now. We can standardise on nf-core and come back to this if needed.

ewels added a commit that referenced this pull request Sep 20, 2024
See #5314 (comment)

Signed-off-by: Phil Ewels <phil.ewels@seqera.io>
@ewels
Copy link
Member

ewels commented Sep 20, 2024

👉🏻 #5316

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.

Additional manifest config fields
4 participants