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.

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

docs: RFC on C++17 minimum standard support #4566

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

Conversation

nilason
Copy link
Contributor

@nilason nilason commented Oct 21, 2024

Addresses #4040.

For reference:
GDAL RFC98
PDAL 2.4
GEOS#1144

@nilason nilason added this to the 8.5.0 milestone Oct 21, 2024
@github-actions github-actions bot added the RFC Request For Comment (RFC) document label Oct 21, 2024
@echoix
Copy link
Member

echoix commented Oct 21, 2024

Is there a way to write it such that the RFC doesn't have to be updated at each version? Such as the conditions that allow us to say: these are the supported ones without having to think more about it?

For Python, the case is quite easy.

@nilason
Copy link
Contributor Author

nilason commented Oct 21, 2024

Is there a way to write it such that the RFC doesn't have to be updated at each version? Such as the conditions that allow us to say: these are the supported ones without having to think more about it?

For Python, the case is quite easy.

I can only repeat what I said before on this matter: #4040 (comment).

@wenzeslaus
Copy link
Member

@echoix I would prefer the Python way as well, but I'm afraid @nilason is right that it is just more complicated for C and C++. Maybe it will change it in the future, but the nature of the ecosystem is just very different now.

One thing which we could do differently is updating the same RFC instead of creating a new one. I don't know if that's a good idea here or in general. For things like release schedule it seems to make sense, though.

@nilason
Copy link
Contributor Author

nilason commented Oct 24, 2024

It is probable we will want to update C and C++ minimum standard only every 5–10 years, so a new RFC weighing in all factors is not such a big deal I think. The current standard should be mentioned in doc/development/style_guide.md.

For reference eg. GDAL RFCs work in the same way, without "updating". Each RFC is a document in itself, with its history and adoption procedure.

@wenzeslaus
Copy link
Member

@nilason, do you want to make any changes to tested C and C++ version in relation to this? The gcc.yml workflow is already using gnu17 and c++17, but perhaps adding new versions? I don't know what GCC is there, but the underlying image is currently ubuntu-22.04. Related to that, do you want to run the CI with clang? macOS seems like a good image to do that right now.

Copy link
Member

@wenzeslaus wenzeslaus left a comment

Choose a reason for hiding this comment

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

I have some wording suggestions, but otherwise we are only testing C++17 in gcc.yml and our dependencies already require C++17, so I don't think there is much to discuss.

@nilason
Copy link
Contributor Author

nilason commented Oct 25, 2024

I have some wording suggestions, but otherwise we are only testing C++17 in gcc.yml and our dependencies already require C++17, so I don't think there is much to discuss.

Thanks for the good suggestions. In practice, also the mac runner is C++17, as that is the default with Clang 19.

wenzeslaus
wenzeslaus previously approved these changes Oct 26, 2024
Copy link
Member

@wenzeslaus wenzeslaus left a comment

Choose a reason for hiding this comment

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

If I recall correctly, the procedure is that the PSC needs to vote here by approving the PR. Please, check PSC mailing list for the last RFC voting.

I guess we could merge the draft and then only PSC-vote to "un-draft" it. I don't see much advantage in that.

@nilason nilason changed the title docs: RFC on C++17 minimal standard support docs: RFC on C++17 minimum standard support Oct 30, 2024
veroandreo
veroandreo previously approved these changes Oct 30, 2024
Copy link
Contributor

@veroandreo veroandreo left a comment

Choose a reason for hiding this comment

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

Thanks @nilason!

@nilason nilason dismissed stale reviews from veroandreo and wenzeslaus via 107a90b October 30, 2024 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFC Request For Comment (RFC) document
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants