Skip to content

Conversation

@urbanjost
Copy link
Contributor

When trying to update #364 all I got was "Sorry, an error occurred" so this is a reconstituted pull request. See #364 and #362 for previous comments.

Copy link
Member

@LKedward LKedward left a comment

Choose a reason for hiding this comment

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

I approved the original PR, and this all looks good to me 👍. I will however second Sebastian's previous comment about documenting the response file functionality which would be good.

@urbanjost
Copy link
Contributor Author

The response file concept was part of an earlier PR proposing a --flag option which was reduced to just the --compiler option and a suggestion from @awvwgk. It allowed me to experiment with several concepts concerning an ealier --profile proposal and is useful but I decided at the time that it was probably better to put the profiles into the manifest itself with a compiler and os and flag list comprising a profile, and then (since the manifest file has no conditional capabilities) that fpm could then just choose based on current compiler and OS and profile name so the logic incorporated into fpm would not be too complex. As-is it does not quite do what I want with a subcommand, is not recursive, only uses the last specification of an option specified multiple times, which I originally planned on changing (and a few other things) and can select between options based on OS but not on compiler it has limitations but is useful so I put a basic description in of the simplest use case with a reference to the M_CLI2 github repository. I originally proposed it as a prototype for discussion but with the other solutions for a package-able custom build proceeding and being part of the GSOC and this being a "free" feature from M_CLI2 I think it can proceed as a supplemental feature as-is(?)

Copy link
Member

@awvwgk awvwgk left a comment

Choose a reason for hiding this comment

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

Thanks, this really looks great now.

@LKedward
Copy link
Member

LKedward commented Jun 5, 2021

Including the original PR (#364), we have four approvals so I'll now merge. Thanks @urbanjost

@LKedward LKedward merged commit 87a2cbf into fortran-lang:master Jun 5, 2021
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.

3 participants