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

issue 3099 - C++17ify #3119

Closed
wants to merge 1 commit into from
Closed

issue 3099 - C++17ify #3119

wants to merge 1 commit into from

Conversation

LenWilliamson
Copy link

Added features.

@LenWilliamson
Copy link
Author

Okay, I forget to remove some merge conflict markers. I was only looking in the files I did changes. I fixed it and run make again. Then I will run core_test. But my laptop (not the most powerful machine) often crashes while running make core_test -j. I don't know why...

@wezrule
Copy link
Contributor

wezrule commented Mar 8, 2021

Thanks! Looks like there are still some merging issues (std::mutex should be nano::mutex for instance). Also clicking Details on our CI checks will show there are some no discard checks being missed. In some cases where the result is actually not cared about it can be cast to (void).

@wezrule
Copy link
Contributor

wezrule commented Mar 8, 2021

I don't think we should have [[nodiscard]] in the source files too, header file is sufficient. I was originally thinking of just having it on functions that return error codes which might accidentally not get checked, but maybe having them on all (like you've done) is more appropriate

@LenWilliamson
Copy link
Author

LenWilliamson commented Mar 13, 2021

Hi, I will check Details. Also I will remove [[nodiscard]] in the .cpp files. Okay, if there is anything else let me know.

@LenWilliamson
Copy link
Author

Regarding the [[nodiscard]] part, if you think having it on all is more appropriate than it is fine. But to be consistent one would have to put [[nodiscard]] in front of every function that returns bool. And I don't know if this adds the desired readability? I'm currently still very inexperienced. In my opinion it would be better to put [[nodiscard]] on functions that return error codes, as you originally thought. For instance, why shouldn't one want to discard nano::state_block::operator==. It doesn't cause me any trouble to revert these changes. Maybe you can give me an example of a function where it truly makes sense to put a [[nodiscard]] attribute. I would think in nano/lib/logger_mt.hpp bool try_log maybe?

@LenWilliamson
Copy link
Author

  1. Is it not sufficient to execute /nano-node$ ci/clang-format-all.sh for the formatting?
  2. Why are so many tests failing? When I rerun the test on my computer I got:

grafik

@anishsingh935
Copy link

Can I fix this problem ??

@anishsingh935
Copy link

Please

@LenWilliamson
Copy link
Author

Yes, please. It would be great if you could also share the reason for the problem and how you fixed it. So I don't run into it again.

@zhyatt zhyatt added this to the V23.0 milestone Apr 27, 2021
This pull request was closed.
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

6 participants