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.

Sign up for GitHub

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

Jump to bottom

Modernize the codebase - and, or, not #2498

Merged
merged 13 commits into from
Oct 19, 2022
Merged

Modernize the codebase - and, or, not #2498

merged 13 commits into from
Oct 19, 2022

Conversation

JanVogelsang
Copy link
Contributor

In this PR all occurrences of &&, || and ! are changed to and, or and not respectively.

Additionally, all redundant parentheses around boolean expressions are removed.

e.g.: if ( ( a == 1 ) && ( b == 2 ) ) was changed to if ( a == 1 and b == 2 )

@JanVogelsang
Copy link
Contributor Author

Unfortunately, two of the checks do not pass yet, because of some SLI regression test failing. I went over every single of my changes in this PR multiple times now and could not find any change that would actually change the compiled code in any way. All changes are purely style-based. Either I am missing something, or there is some reason related to SLI that I am not aware of, that causes the checks to fail. Could someone assist me here?

@heplesser
Copy link
Contributor

@JanVogelsang I can at least explain why only two of the tests fail: The passing ones include MPI, and the test is skipped for MPI (for technical reasons). Can you reproduce the test failure on your computer if you build without MPI?

@JanVogelsang
Copy link
Contributor Author

I can reproduce the failing tests on my computer without building with MPI. Same error

heplesser
heplesser requested changes Oct 13, 2022
View reviewed changes
Copy link
Contributor

@heplesser heplesser left a comment

Choose a reason for hiding this comment

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

I think I found the error, see suggestion.

sli/tokenstack.h Outdated Show resolved Hide resolved
@JanVogelsang JanVogelsang marked this pull request as ready for review October 13, 2022 19:48
@heplesser heplesser requested a review from jougs October 14, 2022 05:52
@heplesser heplesser mentioned this pull request Oct 14, 2022
Merged
jougs
jougs reviewed Oct 17, 2022
View reviewed changes
Copy link
Contributor

@jougs jougs left a comment •
edited

Choose a reason for hiding this comment

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

I guess the replacement of the operators is unproblematic here. I still have a bunch of other comments regarding the statements in the lines that change. Most of my comments also apply to other places in the code in addition to the ones I have marked.

models/ac_generator.cpp Show resolved Hide resolved
libnestutil/lockptr.h Show resolved Hide resolved
models/aeif_psc_alpha.cpp Show resolved Hide resolved
models/noise_generator.cpp Show resolved Hide resolved
sli/tarrayobj.cc Outdated Show resolved Hide resolved
sli/sliarray.cc Outdated Show resolved Hide resolved
sli/sliarray.cc Outdated Show resolved Hide resolved
sli/sliarray.cc Outdated Show resolved Hide resolved
sli/slicontrol.cc Outdated Show resolved Hide resolved
sli/sligraphics.cc Outdated Show resolved Hide resolved
@JanVogelsang JanVogelsang mentioned this pull request Oct 17, 2022
Merged
jougs
jougs approved these changes Oct 19, 2022
View reviewed changes
Copy link
Contributor

@jougs jougs left a comment

Choose a reason for hiding this comment

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

Thanks for taking care of this. It is really much appreciated :-)

JanVogelsang and others added 2 commits October 19, 2022 11:20
@jougs jougs merged commit 7bdb996 into nest:master Oct 19, 2022
@jougs jougs changed the title Modernizing the codebase - and, or, not Modernize the codebase - and, or, not Oct 19, 2022
@jougs jougs added S: Normal Handle this with default priority T: Maintenance Work to keep up the quality of the code and documentation. I: Internal API Changes were introduced in basic internal workings of the simulator that developers need to know labels Oct 19, 2022
@jougs jougs added this to PRs in progress in Kernel via automation Oct 19, 2022
@JanVogelsang JanVogelsang deleted the refactoring-and-or-not branch November 29, 2022 14:59
@terhorstd terhorstd moved this from PRs in progress to Done (PRs and issues) in Kernel Jan 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@heplesser heplesser heplesser approved these changes

@jougs jougs jougs approved these changes

Assignees
No one assigned
Labels
I: Internal API Changes were introduced in basic internal workings of the simulator that developers need to know S: Normal Handle this with default priority T: Maintenance Work to keep up the quality of the code and documentation.
Projects
Kernel
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@JanVogelsang @heplesser @jougs