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

[Metal] Silence compile warning with [[maybe_unused]] #650

Merged
merged 2 commits into from
Mar 24, 2020

Conversation

k-ye
Copy link
Member

@k-ye k-ye commented Mar 24, 2020

@k-ye k-ye requested a review from taichi-gardener March 24, 2020 12:58
@yuanming-hu
Copy link
Member

yuanming-hu commented Mar 24, 2020

Does requesting a review from @taichi-gardener (which is a robot) means a request to auto-format your code (which sounds like a great idea), or should I review it instead?

@k-ye
Copy link
Member Author

k-ye commented Mar 24, 2020

Does requesting a review from @taichi-gardener (which is a robot) means a request to auto-format your code (which sounds like a great idea), or should I review it instead?

The former :) I will ping you directly if i think a PR is worth a review... BTW, I found that clicking the format server sometimes didn't generate a reformat commit. Does that mean that there is no change after the server formats the PR?

@yuanming-hu
Copy link
Member

The former :)

This sounds like a great feature to implement. Much better than the current format server approach.

BTW, I found that clicking the format server sometimes didn't generate a reformat commit. Does that mean that there is no change after the server formats the PR?

Right, there's no way to commit without a changeset.

@k-ye
Copy link
Member Author

k-ye commented Mar 24, 2020

This sounds like a great feature to implement. Much better than the current format server approach.

I should probably clarify 😂 I added @taichi-gardener in the hope that it will do an automatic reformat for me on this PR. Not sure if this actually works...

@yuanming-hu
Copy link
Member

This sounds like a great feature to implement. Much better than the current format server approach.

I should probably clarify I added @taichi-gardener in the hope that it will do an automatic reformat for me on this PR. Not sure if this actually works...

No, currently this won't work. But I think this is a great idea. Someone needs to implement this using github API.

@k-ye
Copy link
Member Author

k-ye commented Mar 24, 2020

Hmm, guess this will be one of those "nuance differences" between my local clang-format (9.0 IIRC) and clang-format-6.0... Seems like 6.0 couldn't handle the code nested inside the STR macro very well. I would probably just force repush..

@yuanming-hu
Copy link
Member

yuanming-hu commented Mar 24, 2020

Sounds good. Another option is to install clang-format-9.0 on the format server when it is easy enough to use.

@k-ye
Copy link
Member Author

k-ye commented Mar 24, 2020

I guess one of those clang-format options probably controls this behavior, but i'm too lazy to figure that out... Anyway, I've reformatted locally and disabled clang-format around this piece of code

@k-ye k-ye merged commit f8fd28c into taichi-dev:master Mar 24, 2020
@k-ye k-ye deleted the unused branch March 24, 2020 23:04
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.

2 participants