Dead code removal in client/src #51
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nedbass
force-pushed
the
b_rm_dead_code
branch
from
1500384
to
321ac70
Compare
nedbass
requested review from
adammoody,
dsikich,
sandrain,
kathrynmohror and
jgmoore-or
Update checkpatch.pl to support four-space indentation since that is the convention used in this project. Otherwise we get false warnings like this: WARNING: suspect code indent for conditional statements (8, 12) LLNL#310: FILE: client/src/unifycr-stdio.c:2523: + if (p > buf) { (void) ungetc(*(u_char *)--p, (FILE *)fp);
Remove commented out code blocks and fix a few code formatting issues in client/src/unifycr-stdio.c. This is to improve code readability.
Remove commented out code in client/src/unifycr.c to improve readability. This leaves behind stubbed out functions that presumably need to be implemented. We will open github issues to track that work and link to this commit in case the removed code is needed for reference.
nedbass
force-pushed
the
b_rm_dead_code
branch
from
321ac70
to
8165103
Compare
adammoody
approved these changes
Dec 21, 2017
|
This generally looks fine to me. One note about the memcopy. We have a few variants of that around because it is handy for testing when saving data in shared memory. We found that different memcopy functions performed quite differently depending on the architecture and compiler. I think for testing I was lazy and just commented them in/out. We should find a good way to do that if it still matters, e.g., compile time guards or something. |
nedbass
added a commit
to nedbass/UnifyCR
that referenced
this pull request
Dec 23, 2017
Commit 75543ec ("Dead code removal in client/src (LLNL#51)")' made checkpatch.pl unfriendly toward tab-indented code. We haven't completely converted to the 4-space indentation convention, so revert that part of the commit. Otherwise we get false warnings like this: WARNING: suspect code indent for conditional statements (8, 16) LLNL#497: FILE: meta/src/mdhim.c:799: if (op == MDHIM_GET_EQ || op == MDHIM_GET_PRIMARY_EQ) { + mlog(MDHIM_CLIENT_CRIT, "MDHIM Rank: %d - "
nedbass
added a commit
to nedbass/UnifyCR
that referenced
this pull request
Dec 27, 2017
Commit 75543ec ("Dead code removal in client/src (LLNL#51)")' made checkpatch.pl unfriendly toward tab-indented code. We haven't completely converted to the 4-space indentation convention, so revert that part of the commit. Otherwise we get false warnings like this: WARNING: suspect code indent for conditional statements (8, 16) LLNL#497: FILE: meta/src/mdhim.c:799: if (op == MDHIM_GET_EQ || op == MDHIM_GET_PRIMARY_EQ) { + mlog(MDHIM_CLIENT_CRIT, "MDHIM Rank: %d - "
kathrynmohror
pushed a commit
that referenced
this pull request
Jan 3, 2018
Commit 75543ec ("Dead code removal in client/src (#51)")' made checkpatch.pl unfriendly toward tab-indented code. We haven't completely converted to the 4-space indentation convention, so revert that part of the commit. Otherwise we get false warnings like this: WARNING: suspect code indent for conditional statements (8, 16) #497: FILE: meta/src/mdhim.c:799: if (op == MDHIM_GET_EQ || op == MDHIM_GET_PRIMARY_EQ) { + mlog(MDHIM_CLIENT_CRIT, "MDHIM Rank: %d - "
This pull request was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Remove a bunch of commented out code and address a few nearby code formatting issues.
I updated checkpatch.pl in this PR because it doesn't correctly support the indentation convention for this project and it was causing false warnings for the other patches.
Some functions in unifycr.c are now completely stubbed out. Rather than keeping the code commented out there, I propose we create github issues to track the need to implement (or possibly remove) the stubs.