Skip to content

Variable string #60

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

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Variable string #60

wants to merge 12 commits into from

Conversation

DavidAKopriva
Copy link
Collaborator

When FTOL was first written, gfortran did not have allocatable strings implemented yet, so a workaround was used where a requested length parameter would set the string length for returns from stringValue. This is now removed (finally) since fortran includes the functionality now.

Remove the "requestedLength" arguments and use allocatable strings now that these are available.
The readme is updated. Add users guide in markdown format and remove latex version.
Remove more dependences on fixed length character variables.
Add links to examples. Add info about optData functionality to the documentation.
Copy link

codecov bot commented Apr 15, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.50%. Comparing base (061999f) to head (3b87d01).

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #60   +/-   ##
=======================================
  Coverage   94.50%   94.50%           
=======================================
  Files          32       32           
  Lines        2820     2820           
=======================================
  Hits         2665     2665           
  Misses        155      155           
Flag Coverage Δ
unittests 94.50% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@andrewwinters5000 andrewwinters5000 left a comment

Choose a reason for hiding this comment

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

The code simplifications look good. My comments are concentrated into the new User Guide. Overall, converting the LaTeX documentation into markdown is very good. There seem to be some slight spacing inconsistencies across the example code blocks.

Align code blocks.
More code formatting. Fix link to Apple.
Caught a couple more alignment errors.
Copy link
Member

@andrewwinters5000 andrewwinters5000 left a comment

Choose a reason for hiding this comment

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

Everything looks good, just one last question.

Fix an escape for a single underscore.
Copy link
Member

@andrewwinters5000 andrewwinters5000 left a comment

Choose a reason for hiding this comment

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

LGTM! This is good to merge. We just need to coordinate with analogous changes in HOHQMesh for the breaking change

@DavidAKopriva
Copy link
Collaborator Author

Right. Let's do your doc update PR first, since it's simple. Then let's discuss how to coordinate the HOHQMesh update.

@andrewwinters5000
Copy link
Member

Yes, I think that is the best order of operations. Then we can make a new release of HOHQMesh that contains the FToL changes and add an appropriate NEWS.md to HOHQMesh to inform all users.

DavidAKopriva and others added 3 commits April 21, 2025 09:26
Fix code examples formatting.
Fix code example object names.
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.

2 participants