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

Make more function parameters const #4033

Closed
daverodgman opened this issue Jan 14, 2021 · 6 comments
Closed

Make more function parameters const #4033

daverodgman opened this issue Jan 14, 2021 · 6 comments
Assignees
Labels
enhancement good-first-issue Good for newcomers size-l Estimated task size: large (2w+)

Comments

@daverodgman
Copy link
Contributor

daverodgman commented Jan 14, 2021

Change the public interface function parameters to be const, where appropriate.

Adding const to some internal functions is not part of this task, as it can be done at any time after 3.0.

It's recommended to submit PRs for individual modules or areas of code rather than one large PR.

@mstarzyk-mobica
Copy link
Contributor

I'll take it as my first task

@mpg
Copy link
Contributor

mpg commented Feb 1, 2021

There were other github issues on the same theme, I'm linking them here for the sake of completeness:

I'll comment directly on the relevant issues why I think some of them are invalid, but first I wanted to add the links here to make sure this is tracked.

@mpg
Copy link
Contributor

mpg commented Feb 1, 2021

Ok, I've now commented on the issues and PRs linked directly or indirectly from the above comment. The only one I didn't think was invalid or unwanted to other reasons was #396 for which you might want to create a PR (the existing PR addressing it also contains changes that I don't think we want, as I commented there).

@mpg
Copy link
Contributor

mpg commented Feb 5, 2021

Note: once the PRs for each module have been merged*, it would be good to add a ChangeLog entry (perhaps under "API changes") describing the changes briefly. I'm suggestion a single ChangeLog entry as this is relatively low-impact and we don't want to overwhelm users with details (especially as the 3.0 ChangeLog will contain more important things).

*Alternatively, a PR for the ChangeLog entry can be created before the other PRs are merged, but labeled "needs: preceding PR" (with references in the description) in order to reduce latency.

@gilles-peskine-arm
Copy link
Contributor

@mpg Good point about changelogs. #4114

@mpg
Copy link
Contributor

mpg commented Feb 16, 2021

I think the following PRs (all merged by now) collectively resolve this issue: #4063 (aria), #4063 (asn1), #4066 (asn1write), #4102 (PK parse), #4115 (ChangeLog), unless anyone sees any more work to do here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement good-first-issue Good for newcomers size-l Estimated task size: large (2w+)
Projects
None yet
Development

No branches or pull requests

5 participants