Skip to content

docs: added documentation for preprocess table #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.

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
Closed
wants to merge 1 commit into from
Closed

docs: added documentation for preprocess table #60

wants to merge 1 commit into from

Conversation

arteevraina
Copy link
Member

In this PR, I have added documentation for preprocess table that can be defined in fpm.toml file.

awvwgk
awvwgk reviewed Jul 5, 2022
View reviewed changes
awvwgk
awvwgk reviewed Jul 5, 2022
View reviewed changes
awvwgk
awvwgk reviewed Jul 5, 2022
View reviewed changes
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.

Should we encourage the usage of dotted keys?

[preprocess]
cpp.suffixes = ["F90", "f90"]
cpp.directories = ["src/feature1", "src/models"]
cpp.macros = ["-DMACRO1", "-DMACRO2"]

awvwgk
awvwgk reviewed Jul 5, 2022
View reviewed changes
@arteevraina
Copy link
Member Author

Should we encourage the usage of dotted keys?

[preprocess]
cpp.suffixes = ["F90", "f90"]
cpp.directories = ["src/feature1", "src/models"]
cpp.macros = ["-DMACRO1", "-DMACRO2"]

Should we encourage the usage of dotted keys?

[preprocess]
cpp.suffixes = ["F90", "f90"]
cpp.directories = ["src/feature1", "src/models"]
cpp.macros = ["-DMACRO1", "-DMACRO2"]

Indeed ! We can add a paragraph showing this way of writing the toml file as well. Will update this in the PR.

milancurcic
milancurcic approved these changes Jul 11, 2022
View reviewed changes
Copy link
Member

@milancurcic milancurcic left a comment

Choose a reason for hiding this comment

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

Very nice, thank you. I made some editorial suggestions for improved grammar and more active voice.

pages/spec/manifest.md Outdated
```toml
[preprocess]
[preprocess.cpp]
macros = ["FOO", "BAR"]
Copy link
Member

Choose a reason for hiding this comment

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

Just a note that we should think about the manifest syntax to set preprocessor macros to specific values, rather than just defined/undefined. For example, how to set -DPHYSICS_OPTION=2 or similar.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I will try to design a syntax for this and try to accommodate it with what we currently have. I will send a note on discourse when done.

Copy link
Member

Choose a reason for hiding this comment

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

Note that a common macro would be -DVERSION=..., which is using information from the package manifest.

@arteevraina
Copy link
Member Author

Thanks for reviewing @milancurcic. I have made changes as per the suggestions.

@awvwgk
Copy link
Member

awvwgk commented Sep 7, 2022

Is this PR ready to go?

@arteevraina
Copy link
Member Author

Is this PR ready to go?

I think I will update this with few minor pieces of documentation changes for named macros as well and how we can reuse the version number inside macros list.

@arteevraina
Copy link
Member Author

Is this PR ready to go?

This PR is ready now. @awvwgk

awvwgk
awvwgk reviewed Sep 8, 2022
View reviewed changes
@arteevraina
Copy link
Member Author

Is this PR ready to go?

This PR is ready now. @awvwgk

Resolved the requested changes as well.

@awvwgk awvwgk closed this Sep 8, 2022
@awvwgk awvwgk force-pushed the preprocessor-table branch from 3d871f2 to 3a7f8e9 Compare September 8, 2022 11:54
@awvwgk
Copy link
Member

awvwgk commented Sep 8, 2022

Sorry, tried to update the branch and messed something up. Merged with commit fb0117e.

@arteevraina arteevraina deleted the preprocessor-table branch September 8, 2022 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@milancurcic milancurcic milancurcic approved these changes

@awvwgk awvwgk awvwgk approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@arteevraina @awvwgk @milancurcic