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.

Sign up for GitHub

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

Jump to bottom

EVA-3226 - Map errors to Spreadsheet location #10

Merged
merged 4 commits into from
Sep 11, 2023
Merged

EVA-3226 - Map errors to Spreadsheet location #10

merged 4 commits into from
Sep 11, 2023

Conversation

tcezard
Copy link
Member

@tcezard tcezard commented Sep 7, 2023 •
edited
Loading

I Adapted the report to use the spreadsheet location.
You can view it here

@tcezard
Map errors to Spreadsheet location
705ebc5 
Adapt report to use the spreadsheet location
@tcezard tcezard requested a review from Dona094 September 8, 2023 09:46
@tcezard tcezard mentioned this pull request Sep 8, 2023
Merged
Dona094
Dona094 approved these changes Sep 8, 2023
View reviewed changes
Copy link
Contributor

@Dona094 Dona094 left a comment

Choose a reason for hiding this comment

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

The metadata validation table looks great. It now looks easier to investigate/navigate between the errors of the different sheets for the submitter. The addition of the row-column is also super helpful. Thank you 👍

sundarvenkata-EBI
sundarvenkata-EBI reviewed Sep 10, 2023
View reviewed changes
cli/reporter.py Outdated Show resolved Hide resolved
sundarvenkata-EBI
sundarvenkata-EBI reviewed Sep 10, 2023
View reviewed changes
cli/reporter.py Outdated Show resolved Hide resolved
sundarvenkata-EBI
sundarvenkata-EBI approved these changes Sep 10, 2023
View reviewed changes
Copy link
Member

@sundarvenkata-EBI sundarvenkata-EBI left a comment

Choose a reason for hiding this comment

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

Looks good! Left a few comments that can be optionally addressed.

@tcezard tcezard merged commit 5505724 into EBIvariation:main Sep 11, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nitin-ebi nitin-ebi nitin-ebi approved these changes

@Dona094 Dona094 Dona094 approved these changes

@sundarvenkata-EBI sundarvenkata-EBI sundarvenkata-EBI approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@tcezard @sundarvenkata-EBI @Dona094 @nitin-ebi