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

SSF for 2019_Haber_Crusaders #153

Merged
merged 5 commits into from
Feb 9, 2024
Merged

SSF for 2019_Haber_Crusaders #153

merged 5 commits into from
Feb 9, 2024

Conversation

93Boy
Copy link
Contributor

@93Boy 93Boy commented Jan 18, 2024

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 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.

  • 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).

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.

Look good, from what I can see, and Validation passes. From my end good to. @AyGhal ?

@nevrome
Copy link
Member

nevrome commented Jan 22, 2024

  • The checklists above are used incorrectly. The new package list does not apply here at all and the modifying checklist is filled only partially. I will explain these checklists once more in our next meeting on Friday, as they seem to be a major point of confusion.
  • The .ssf file added here has a Poseidon_IDs, but not the necessary poseidon_IDs column. That means the file is not properly linked to the package. The column must be renamed. This issue is reported in the GitHub action trident validate log:

[Warning] [2024-01-18 19:40:55] Potential consistency issues in file data/2019_Haber_Crusaders/ENAtable.ssf: The poseidon_IDs column is completely empty. Package and .ssf file are not linked

@stschiff
Copy link
Member

Oh man, that's embarassing for me, as I've signed this off before. OK, @93Boy, please

  1. go through the PR text above (take a look at the raw Markdown, where Clemens have put helpful comments) and remove the bits that are irrelevant for an existing package.
  2. Make the required changes to the SSF file as indicated by Clemens.
  3. I've marked this as Draft. Please mark it for review whenever you're ready.

@stschiff stschiff marked this pull request as draft January 23, 2024 09:34
@93Boy 93Boy marked this pull request as ready for review January 25, 2024 18:43
@stschiff
Copy link
Member

@AyGhal this is good to merge, please!

@AyGhal
Copy link
Contributor

AyGhal commented Feb 9, 2024

Looks great!

@AyGhal AyGhal merged commit 234d166 into master Feb 9, 2024
1 check passed
@AyGhal AyGhal deleted the 2019_Haber_Crusaders_ssf branch February 9, 2024 08:35
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.

4 participants