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

(Closes #350) disable validation checks by default #351

Merged
merged 2 commits into from
Jun 16, 2022

Conversation

arporter
Copy link
Member

No description provided.

@codecov
Copy link

codecov bot commented Jun 15, 2022

Codecov Report

Merging #351 (fd35907) into master (ffee889) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #351   +/-   ##
=======================================
  Coverage   91.34%   91.34%           
=======================================
  Files          36       36           
  Lines       13047    13052    +5     
=======================================
+ Hits        11918    11923    +5     
  Misses       1129     1129           
Impacted Files Coverage Δ
src/fparser/two/symbol_table.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ffee889...fd35907. Read the comment docs.

@arporter
Copy link
Member Author

@TeranIvy this is the PR for the fparser fix. HEAD of PSyclone is fine with this branch and all of the NEMO source parses without errors. I've just tried it on LFRic too and it seems fine.

@arporter arporter self-assigned this Jun 15, 2022
@arporter
Copy link
Member Author

This is ready for review now.

@TeranIvy TeranIvy linked an issue Jun 15, 2022 that may be closed by this pull request
@TeranIvy TeranIvy mentioned this pull request Jun 15, 2022
@TeranIvy
Copy link
Collaborator

Tested in the LFRic suite. All green: PSycloning, compilation, runs, style checks and KGO checks.

@arporter
Copy link
Member Author

Tested in the LFRic suite. All green: PSycloning, compilation, runs, style checks and KGO checks.

Hi @TeranIvy, was that with HEAD of PSyclone master?

@TeranIvy
Copy link
Collaborator

TeranIvy commented Jun 16, 2022

Tested in the LFRic suite. All green: PSycloning, compilation, runs, style checks and KGO checks.

Hi @TeranIvy, was that with HEAD of PSyclone master?

Yes, @arporter, with the head of PSyclone master. I created a PSyclone branch from the head of master with the fparser submodule from this branch and then I installed PSyclone test environment from that.

BTW, that was yesterday (15 June) evening. I see that there is a fresh merge to head. Shall I test it again?

@sergisiso
Copy link
Collaborator

@TeranIvy The last changes shouldn't change LFRic behavior but do touch the base classes that LFRic use. So it would be safer to test again.

Copy link
Collaborator

@rupertford rupertford left a comment

Choose a reason for hiding this comment

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

Changes look good. pylint and pycodestyle pass. No need to document at this point. Approving.

@rupertford rupertford merged commit b37026c into master Jun 16, 2022
@rupertford rupertford deleted the 350_disable_checks branch June 16, 2022 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow symbol checks to be disabled
4 participants