Skip to content

fprettify#229

Merged
ArshSaja merged 11 commits intomainfrom
fprettify
Feb 20, 2023
Merged

fprettify#229
ArshSaja merged 11 commits intomainfrom
fprettify

Conversation

@sseraj
Copy link
Copy Markdown
Collaborator

@sseraj sseraj commented Aug 18, 2022

Purpose

Formatting the Fortran code with fprettify

Expected time until merged

1 week

Type of change

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (non-backwards-compatible fix or feature)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Documentation update
  • Maintenance update
  • Other (please describe)

Testing

Current tests pass

Checklist

  • I have run flake8 and black to make sure the code adheres to PEP-8 and is consistently formatted
  • I have run unit and regression tests which pass locally with my changes
  • I have added new tests that prove my fix is effective or that my feature works
  • I have added necessary documentation

@sseraj sseraj requested a review from a team as a code owner August 18, 2022 23:07
@sseraj
Copy link
Copy Markdown
Collaborator Author

sseraj commented Aug 18, 2022

fprettify is failing because of this issue fortran-lang/fprettify#93
The issue has been fixed but is not available on the latest tag on pyPI.

@codecov
Copy link
Copy Markdown

codecov bot commented Aug 18, 2022

Codecov Report

Merging #229 (3ba85fe) into main (92fb234) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #229   +/-   ##
=======================================
  Coverage   40.77%   40.77%           
=======================================
  Files          13       13           
  Lines        3882     3882           
=======================================
  Hits         1583     1583           
  Misses       2299     2299           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ewu63
Copy link
Copy Markdown
Collaborator

ewu63 commented Aug 19, 2022

fprettify is failing because of this issue pseewald/fprettify#93 The issue has been fixed but is not available on the latest tag on pyPI.

Do you wanna open an issue there to ask if they can make a new release?

@sseraj
Copy link
Copy Markdown
Collaborator Author

sseraj commented Aug 22, 2022

I am converting this PR into a draft until we merge another PR that removes label statements. Labels are not considered best practice for F90 and cause problems with fprettify.

@sseraj sseraj marked this pull request as draft August 22, 2022 19:26
@ewu63
Copy link
Copy Markdown
Collaborator

ewu63 commented Sep 20, 2022

@sseraj are the labels changed in #228? I just don't want this to get too stale since this potentially blocks all Fortran development.

@sseraj
Copy link
Copy Markdown
Collaborator Author

sseraj commented Sep 21, 2022

The labels are not changed in #228. I will open another PR for the label fixes, but I have not had time to work on that lately. I will try to have the labels fixed by the time we merge #231, because we want to merge that PR before this one anyway.

@sseraj sseraj mentioned this pull request Dec 16, 2022
13 tasks
@sseraj
Copy link
Copy Markdown
Collaborator Author

sseraj commented Feb 3, 2023

I don't know what's going on with the complex build because both of the last commits work for me locally. For context, I tried changing complexify.py to get it to stop replacing strings like "======" with ".ceq..ceq..ceq.". This was happening even before the fprettify changes but did not cause errors because the line character limit was not being exceeded. With the indenting changes in this PR, the line character limit is being exceeded.

@sseraj sseraj marked this pull request as ready for review February 6, 2023 17:08
@sseraj
Copy link
Copy Markdown
Collaborator Author

sseraj commented Feb 6, 2023

This is ready to be merged. @marcomangano @ArshSaja

The only Fortran file that has manual changes is src/preprocessing/preprocessingAPI.F90. The manual changes were necessary to fix the complex build issue that I mentioned above. I replaced the equal signs with dashes in the strings that were causing problems. Ideally, complexify.py would not change strings at all, but this would involve changes to the parser. I opened an issue for this in the complexify repo, which will eventually replace the current complex build process.

Copy link
Copy Markdown
Collaborator

@marcomangano marcomangano left a comment

Choose a reason for hiding this comment

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

The manual changes are fine. Everything else is assumed fine as the tests pass without issue - the git diff is not helpful unfortunately. Please add the commit hash to .git-blame-ignore-revs asap.
Nice job!

@sseraj
Copy link
Copy Markdown
Collaborator Author

sseraj commented Feb 20, 2023

Yes, I will open another PR updating .git-blame-ignore-revs after this is merged.

@ArshSaja ArshSaja merged commit cd6ec63 into main Feb 20, 2023
@sseraj sseraj deleted the fprettify branch February 20, 2023 19:55
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.

4 participants