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

Fix warnings regarding int-type variables #3026

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix warnings regarding int-type variables #3026

wants to merge 2 commits into from

Conversation

muffgaga
Copy link
Contributor

@muffgaga muffgaga commented Dec 1, 2023 •
edited
Loading

This PR fixes build-time warnings from the compiler that I encountered in my local build.
It cleans up comparisons between unsigned size-like types (e.g. size_t) and signed indices (long/int).

Sorry for the close (of #3025)/reopen, github reacted weird when I pushed with full reference name refs/heads/fix_warnings2…

@terhorstd terhorstd added S: Normal Handle this with default priority T: Maintenance Work to keep up the quality of the code and documentation. I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) labels Dec 15, 2023
heplesser
heplesser requested changes Feb 5, 2024
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.

Thanks for the PR. I added one suggestion to ensure consistency.

models/correlomatrix_detector.cpp Outdated Show resolved Hide resolved
@heplesser heplesser added this to the NEST 3.8 milestone Jun 20, 2024
@heplesser heplesser requested review from JanVogelsang and removed request for ddahmen June 20, 2024 12:06
JanVogelsang
JanVogelsang approved these changes Jun 20, 2024
View reviewed changes
Copy link
Contributor

@JanVogelsang JanVogelsang left a comment

Choose a reason for hiding this comment

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

Looks good to me, as long as the changes by @heplesser get integrated.

@heplesser heplesser removed this from the NEST 3.8 milestone Jun 27, 2024
@terhorstd
Copy link
Contributor

@muffgaga , this seems to be basically complete except for the suggested change. Could you also add a description to this PR? The current sentence seems to have little relation to what your proposed change is.

muffgaga and others added 2 commits July 24, 2024 18:40
@muffgaga
fix: more warnings
226e5a2 
This commit fixes build-time warnings from the compiler that I
encountered in my local build.
It cleans up comparisons between unsigned size-like types (e.g. size_t)
and signed indices (long/int).
@heplesser @muffgaga
fixup(b89293): cleanup nearby variables too
4dfccfb
@muffgaga
Copy link
Contributor Author

@muffgaga , this seems to be basically complete except for the suggested change. Could you also add a description to this PR? The current sentence seems to have little relation to what your proposed change is.

Done, description also added to the commit (wasn't sure if adding text to the PR-opening comment is sufficient).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@heplesser heplesser heplesser requested changes

@JanVogelsang JanVogelsang JanVogelsang approved these changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) S: Normal Handle this with default priority T: Maintenance Work to keep up the quality of the code and documentation.
Projects
Kernel
Status: PRs pending approval
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@muffgaga @terhorstd @heplesser @JanVogelsang