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

utils: display warning messages about REANA spec validation #662

Merged

Conversation

giuseppe-steduto
Copy link
Member

Closes #660

@giuseppe-steduto giuseppe-steduto force-pushed the reana-yaml-allow-unexpected-props branch 3 times, most recently from eedf5fb to 58c36c1 Compare August 10, 2023 12:33
@codecov
Copy link

codecov bot commented Aug 10, 2023

Codecov Report

Merging #662 (769a840) into master (5fc10fb) will decrease coverage by 0.10%.
The diff coverage is 45.45%.

❗ Current head 769a840 differs from pull request most recent head 15a01ad. Consider uploading reports for the commit 15a01ad to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #662      +/-   ##
==========================================
- Coverage   58.63%   58.53%   -0.10%     
==========================================
  Files          24       24              
  Lines        2369     2378       +9     
==========================================
+ Hits         1389     1392       +3     
- Misses        980      986       +6     
Files Changed Coverage Δ
reana_client/validation/utils.py 75.80% <45.45%> (-7.22%) ⬇️

@giuseppe-steduto giuseppe-steduto force-pushed the reana-yaml-allow-unexpected-props branch 2 times, most recently from 6f839aa to 2e76db3 Compare August 24, 2023 14:50
@giuseppe-steduto giuseppe-steduto marked this pull request as ready for review August 24, 2023 14:51
@giuseppe-steduto giuseppe-steduto force-pushed the reana-yaml-allow-unexpected-props branch from 2e76db3 to d857753 Compare August 29, 2023 16:43
Comment on lines 71 to 73
"The REANA specification file is valid, but some warnings were "
"found. Please check the warnings above and make sure that the "
"REANA specification file is correct.",
Copy link
Member

Choose a reason for hiding this comment

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

Given that we already print the warnings before, maybe we can shorten this to just say Please make sure that the REANA specification file is correct?
Or we could also print something like this:

  -> WARNING: Issues found while validating the REANA specification file
  -> WARNING: [... list of warnings ...]
  -> WARNING: Please make sure that the REANA specification file is correct

In any case, what I would like to avoid is to print a very long line in the terminal.

What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay for shortening the single lines printed in the terminal! I think it is still important to write that the REANA specification file appears valid.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe let's add a line to CHANGES.rst?

@giuseppe-steduto giuseppe-steduto force-pushed the reana-yaml-allow-unexpected-props branch 2 times, most recently from 13d6dcd to 769a840 Compare August 30, 2023 13:01
@giuseppe-steduto giuseppe-steduto force-pushed the reana-yaml-allow-unexpected-props branch from 769a840 to 15a01ad Compare August 31, 2023 10:38
@mdonadoni mdonadoni merged commit 15a01ad into reanahub:master Aug 31, 2023
22 checks passed
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.

reana.yaml: consider allowing unexpected properties
2 participants