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

Fixed country naming proposed in #220 #229

Closed
wants to merge 13 commits into from

Conversation

Kavlahkaff
Copy link

@Kavlahkaff Kavlahkaff commented Dec 12, 2024

Changed RussianFederation to Russian Federation, added Country_ISO column, changed .janno file name to be consistent.
Changes were done in connection to this issue

PR Checklist for modifying one or multiple existing packages

  • The changes maintain the structural integrity of the affected packages.
  • The checksums of the modified files in the respective POSEIDON.yml files were adjusted properly.
  • Every file in the submission is correctly referenced in the relevant POSEIDON.yml files and there are no additional, supplementary files in the submission that are not documented there.

  • The packageVersion numbers of the affected packages were increased in their POSEIDON.yml files.
  • The changes in the packageVersion followed the Poseidon Package versioning policy.
  • The changes were documented in the respective CHANGELOG files. If no CHANGELOG files existed previously it was added here.
  • The lastModified fields of the affected POSEIDON.yml files were updated.
  • The contributor fields were updated with name, email and orcid of the relevant, new contributors.
  • The .janno and the .ssf files are not fully quoted, so they only use single- or double quotes ("...", '...') to enclose text fields where it is strictly necessary (i.e. their entry includes a TAB).

  • All affected packages pass a validation with trident validate --fullGeno.

  • Large genotype data files are properly tracked with Git LFS and not directly pushed to the repository. For an instruction on how to set up Git LFS please look here. If you accidentally pushed the files the wrong way you can fix it with git lfs migrate import --no-rewrite path/to/file.bed (see here).

@Kavlahkaff Kavlahkaff self-assigned this Dec 12, 2024
@Kavlahkaff Kavlahkaff marked this pull request as draft December 12, 2024 16:40
@Kavlahkaff Kavlahkaff marked this pull request as ready for review December 12, 2024 17:19
Copy link
Member

@stschiff stschiff left a comment

Choose a reason for hiding this comment

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

OK, great. Thanks! Just out of curiosity, I think there should be a PR template form with various checkboxes that appears when you open a new PR. Have you checked this and removed the form?

In any case I think the changes look all good to me.

@Kavlahkaff
Copy link
Author

Kavlahkaff commented Dec 16, 2024

@stschiff Hi, I removed the template as I was not sure if we keep it when submitting a PR? I just re-read it, and realized I did not check all the boxes. Do I bump up the version number for changes like this?

Edit: I put the template back in the description and checked the boxes which my PR fullfills.

@nevrome
Copy link
Member

nevrome commented Dec 16, 2024

Thanks for bringing the PR checklist back. It should be filled for every PR that changes Poseidon packages in the archive.

The package versions must indeed be incremented following these rules: https://github.com/poseidon-framework/poseidon-schema?tab=readme-ov-file#package-versioning

Remember that you can do all (!) of these changes for all packages at once (!) with a single call to trident rectify.

@Kavlahkaff
Copy link
Author

Kavlahkaff commented Dec 17, 2024

I have now moved this to a new PR directly in this repository.

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.

3 participants