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

Promote many simple const variables to constexpr #4325

Merged
merged 1 commit into from
Sep 27, 2021

Conversation

tcoyvwac
Copy link
Contributor

As encouraged and suggested in Mixxx's Coding Guidelines:


constexpr

Use whenever possible, [...]


https://github.com/mixxxdj/mixxx/wiki/Coding%20Guidelines#constexpr

@Be-ing
Copy link
Contributor

Be-ing commented Sep 27, 2021

Thank you for this nice contribution. Please sign the Contributor Agreement and leave a comment here before we can merge this.

I would prefer to ignore pre-commit's reformatting in this special case to minimize the potential for merge conflicts.

@tcoyvwac
Copy link
Contributor Author

Hello @Be-ing , I would like to sign the Contributor Agreement, but I do not have the ability to use the site. Is there another alternative place where the Agreement can be signed? Or another way to confirm somehow? Thanks for your positive feedback!

@ronso0
Copy link
Member

ronso0 commented Sep 27, 2021

This is the correct link
https://docs.google.com/forms/d/e/1FAIpQLScC9QG327sjLL0eWftmfGUasxFFLxg6LCXJ2xHDYRzFIRqyiw/viewform?formkey=dEpYN2NkVEFnWWQzbkFfM0ZYYUZ5X2c6MQ

@ronso0
Copy link
Member

ronso0 commented Sep 27, 2021

please squash the commits (as long as no in-line review comments have been made).

@tcoyvwac tcoyvwac force-pushed the fix/constexpr-simple-variables branch from b1e632f to edf209a Compare September 27, 2021 02:54
@tcoyvwac
Copy link
Contributor Author

Agreement signed and commits squashed.

@Be-ing
Copy link
Contributor

Be-ing commented Sep 27, 2021

Thank you.

@Be-ing Be-ing merged commit eff27ab into mixxxdj:main Sep 27, 2021
@Be-ing
Copy link
Contributor

Be-ing commented Sep 27, 2021

Please setup pre-commit before starting another branch. I made an exception in this case because the autoformatting would have made this diff way bigger and increased the chance of merge conflicts across many files.

@tcoyvwac
Copy link
Contributor Author

Agreed and very much appreciate the (trade-off) exception @Be-ing! 😄 🎉 For changes like these, I agree, a small / the smallest merge footprint was the aim, and strongly preferred.

Grateful for understanding of what the PR (in context) was trying to achieve. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants