Migrated type defs and changed const to constexpr #1395
Conversation
|
Please see https://github.com/OpenChemistry/avogadrolibs/pull/1395/checks?check_run_id=17973139639 to sign your commits. |
I have signed my commits. |
Signed-off-by: Atharva-Kanherkar <[email protected]>
|
Okay, the buildbots will take over for a bit. Thanks. |
|
What seems to be wrong? How can i fix this? |
|
If you click "Details" on any of the builds / tests, you can see the log, e.g. Looks like you didn't remove the macro consistently. |
Sorry, but i am still confused as the logging errors are way too much in number. I will probably add the macro back again, does it work? |
Signed-off-by: Atharva-Kanherkar <[email protected]>
|
I have written the macro again. |
|
Here are the build results |
|
@ghutchis I do not understand why is this failing the clang-format check, should i not remove anything from the code and just change constexpr? |
|
Is the PR Okay now to be merged? Or any more enhancements are required? |
Atharva-Kanherkar
changed the title
|
While you redefined various things, you also removed the associated comments. Please put them back. |
|
Here are the build results |
Hello, i checked with the original code, it has all the original comments as before. Can you please check again? |
|
That's not true: The diff has a net of -20 lines, almost all comments. |
I am so sorry, i will fix this asap. |
|
I don't see any removed functions, you just moved things to slightly more modern versions which is good. Please don't remove comments, it would be good to update the description to describe it better. Something like "Migrated typedefs and moving const to constexpr". You also removed the final newline in the file and as stated it would be good to get the documentation strings back in there. |
Signed-off-by: Atharva-Kanherkar <[email protected]>
Atharva-Kanherkar
changed the title
Thank you! Can you check now if its all right? I am sorry for responding late, some work popped up. |
|
It looks better to me and the description more accurate as to what was changed. |
|
Here are the build results |
|
Thank you. Is everything okay? |
|
Congrats on merging your first pull request! 🎉 Thanks for making Avogadro better for everyone! |
|
looks great, thanks |
|
Here are the build results |
Developer Certificate of Origin
Version 1.1
Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129
Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.
Developer's Certificate of Origin 1.1
By making a contribution to this project, I certify that:
(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or
(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or
(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.
(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.
I have done the following changes:
1.Used
usinginstead oftypedef ``for type aliases.2.Replaced
constwith` constexpr` for constant values.3.Removed the unnecessary` #if` block related to MSVC.
4.Made` isCustomElement` a` constexpr` function.
5.Used `std::size_t` instead of `size_t.`
6.Removed the macro` AVO_DISABLE_COPY` as it's not needed in modern C++.
I am sorry for the previous deleted pr. There was something wrong with it. I have ensured that this would not only modernize the code, but also remove unnecessary stuff from the code. I am new to open source and any kind of help would be very helpful for me. Thank you! (I am very sorry for the previous prs. There were some issues with my github, this is the final one)