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

Allow trailing commas on macros #390

Merged
merged 1 commit into from Sep 13, 2021
Merged

Allow trailing commas on macros #390

merged 1 commit into from Sep 13, 2021

Conversation

HyeonuPark
Copy link
Contributor

This PR allows to append optional comma after the last argument of those opts!/register! like macros.

@HyeonuPark
Copy link
Contributor Author

I guess all I needs to do for the DCO issue is to append the Signed-off-by: line to the commit msg. Maybe it was a mistake to make the change directly on the GH web editor, which makes it nontrivial to simply change the commit msg :P

@lucab
Copy link
Member

lucab commented Feb 19, 2021

@HyeonuPark thanks for the PR! Indeed, your commit should be signed off to pass CI checks. Travis failure is due to #391 and should go away if you rebase to latest master.

Copy link
Member

@breezewish breezewish left a comment

Choose a reason for hiding this comment

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

Thanks a lot! Could you also supply some unit tests to verify that this syntax pass the compiling? In this way we can avoid some future mistakes :) For example, you can just write a test importing the macro and then use these macros with trailing commas.

@lucab
Copy link
Member

lucab commented Sep 6, 2021

@HyeonuPark ping, do you plan to come back to this soon-ish?

@HyeonuPark
Copy link
Contributor Author

Sorry for the super late reply! If it's still OK, I'll supplement it within this week.

@HyeonuPark
Copy link
Contributor Author

Shameless copy-pastes from doc comments with additional comma per invocation. :) I can't say anymore it's a tiny change w.r.t. the LoC.

Signed-off-by: Hyeonu Park <nemo1275@gmail.com>
@HyeonuPark
Copy link
Contributor Author

Applied cargo fmt on it.

@lucab
Copy link
Member

lucab commented Sep 13, 2021

Thanks a lot, LGTM!

@lucab lucab merged commit 3451d05 into tikv:master Sep 13, 2021
@lucab lucab mentioned this pull request Sep 27, 2021
JanBerktold pushed a commit to JanBerktold/rust-prometheus that referenced this pull request Nov 12, 2022
Signed-off-by: Hyeonu Park <nemo1275@gmail.com>
Signed-off-by: Jan Berktold <jberktold@roblox.com>
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.

None yet

3 participants