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

#2831 Reorder arguments to parser for consistency #2832

Merged
merged 8 commits into from
Jan 9, 2025
Merged

#2831 Reorder arguments to parser for consistency #2832

merged 8 commits into from
Jan 9, 2025

Conversation

mo-alistairp
Copy link
Collaborator

No description provided.

@mo-alistairp mo-alistairp linked an issue Dec 19, 2024 that may be closed by this pull request
Update PSyclone argument definitions to be consistent #2831
Closed
@codecov Codecov
Copy link

codecov bot commented Dec 19, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.88%. Comparing base (93741fd) to head (1355d66).
Report is 9 commits behind head on master.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master    #2832   +/-   ##
=======================================
  Coverage   99.88%   99.88%           
=======================================
  Files         359      359           
  Lines       50264    50264           
=======================================
  Hits        50208    50208           
  Misses         56       56

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mo-alistairp mo-alistairp self-assigned this Dec 19, 2024
@mo-alistairp mo-alistairp added LFRic Issue relates to the LFRic domain in progress and removed LFRic Issue relates to the LFRic domain labels Dec 19, 2024
@mo-alistairp mo-alistairp marked this pull request as ready for review January 6, 2025 15:36
LonelyCat124
LonelyCat124 requested changes Jan 7, 2025
View reviewed changes
Copy link
Collaborator

@LonelyCat124 LonelyCat124 left a comment

Choose a reason for hiding this comment

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

Hi @mo-alistairp

Happy with the changes - I've added one comment to change since I wanted to send it back as you need to add yourself to the Author list in the generator.py file. Can you also update the copyright date in that file to include 2025?

I'll submit integration tests in the mean time.

src/psyclone/generator.py Outdated Show resolved Hide resolved
@LonelyCat124
Copy link
Collaborator

Hi @mo-alistairp - thanks for doing this. For future PRs if you make the changes and comment that you have but leave the comment unresolved as the reviewer will do this.
Also when you send it back for review make sure you add the "ready for review" tag back and remove the reviewed with actions tag.

LonelyCat124
LonelyCat124 approved these changes Jan 9, 2025
View reviewed changes
Copy link
Collaborator

@LonelyCat124 LonelyCat124 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

No changes to generated code so no need to run integration tests again. Ready to merge.

@LonelyCat124 LonelyCat124 merged commit c6e9507 into master Jan 9, 2025
13 checks passed
@LonelyCat124 LonelyCat124 deleted the 2831_PSyclone_Argument_Consistency branch January 9, 2025 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LonelyCat124 LonelyCat124 LonelyCat124 approved these changes

@sergisiso sergisiso Awaiting requested review from sergisiso

@arporter arporter Awaiting requested review from arporter

@TeranIvy TeranIvy Awaiting requested review from TeranIvy

Assignees

@mo-alistairp mo-alistairp

Labels
ready to be merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update PSyclone argument definitions to be consistent
2 participants
@mo-alistairp @LonelyCat124