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

For #1199 #347

Closed
wants to merge 1 commit into from
Closed

For #1199 #347

wants to merge 1 commit into from

Conversation

mugitty
Copy link
Contributor

@mugitty mugitty commented Dec 5, 2023

Note, this PR is associated with geneontology/go-site#2207

@mugitty mugitty requested a review from kltm December 5, 2023 17:43
@dustine32
Copy link
Contributor

@mugitty Change here is to rsync a aggregate-rule-violation-report.md file but where is this file generated? I don't see the command to generate in the Jenkinsfile. Does this line need to be updated too?

sh 'python3 ./scripts/combined_assigned_by.py -v --input ./combined.report.json --output ./assigned-by-combined-report.json'

@mugitty
Copy link
Contributor Author

mugitty commented Dec 5, 2023

aggregate-rule-violation-report.md is an additional file that is generated by combined_assigned_by.py

@dustine32
Copy link
Contributor

@mugitty Oh I see. Filename aggregate-rule-violation-report.md is hard-coded in combined_assigned_by.py.

This works but it might be nicer to parameterize that new output filename so that it's explicitly mentioned in the Jenkinsfile. I'll let @kltm decide whether that's really required.

@kltm
Copy link
Member

kltm commented Dec 5, 2023

When is the file going to start being produced and how? I want to make sure there is no lag so that rsync does not/cannot fail here. Ideally, the file would be produced and in the branches before the move itself was updated.

@mugitty
Copy link
Contributor Author

mugitty commented Dec 5, 2023

@kltm
Copy link
Member

kltm commented Dec 5, 2023

@mugitty it's not being produced on master, snapshot, or release, which are the ones I care about here. A mismatch would cause pipeline failure.

@mugitty
Copy link
Contributor Author

mugitty commented Dec 5, 2023

@kltm , I will update on master

@kltm
Copy link
Member

kltm commented Dec 5, 2023

@mugitty To clarify, the go-site merge needs to happen before the pipeline merge.

@mugitty mugitty closed this Dec 6, 2023
@mugitty mugitty deleted the issue-go-site-1199-aggregate-report branch December 6, 2023 23:08
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