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

fix minor issues found by lgtm scan #2605

Merged
merged 15 commits into from
Jan 5, 2020
Merged

Conversation

grondo
Copy link
Contributor

@grondo grondo commented Jan 4, 2020

Ran across lgtm.com which was already scanning this project, and thought I'd just go ahead and fix some of the obvious errors it caught.

Overall, it seems like lgtm catches some valid (though simple) errors, and it might be nice to turn this service on for pull requests (free for open source projects), if that is ok.

Rename the global Lua state variable from `L` to `global_L` for
clarity and to avoid warnings from lgtm scan about shadowing
global variables.
Copy link
Member

@garlick garlick left a comment

Choose a reason for hiding this comment

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

Just found one typo - everything else is OK. Feel free to set merge-when-passing when that's fixed.
Sounds great on adding to our PR checks.

@@ -8,6 +8,9 @@
* SPDX-License-Identifier: LGPL-3.0
\************************************************************/

#ifndef HAVE_LIBUTIL_CLEANUP_H
#define HAVE_LIBUTIL_CLENAUP_H 1
Copy link
Member

Choose a reason for hiding this comment

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

typo: "CLENAUP"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, thanks. Force pushed a fix. I was kind of hoping lgtm scan would catch that sort of thing, but oh well.

Add header guard to src/common/libutil/cleanup.h as caught during
lgtm scan.
Add header guard to src/common/libutil/mnemonic.h as detected by
lgtm scan.
Add missing header guard to src/common/libutil/unlink_recursive.h
as detected by lgtm scan.
Add missing header guard to src/cmd/builtin.h as detected by lgtm scan.
Add missing header guard to src/broker/content-cache.h as detected by
lgtm scan.
Add missing header guard to src/common/libaggregate/aggregate.h
as detected by lgtm scan.
Add missing header guard to src/broker/liblist.h as detected by
lgtm scan.
Add missing header guard to src/common/libidset/idset_private.h
as detected by lgtm scan.
Add missing header guard to src/broker/pmiutil.h as detected by
lgtm scan.
Add missing header guard to src/common/libschedutil/schedutil_private.h
as detected by lgtm scan.
Add missing header guard to src/shell/mpir/rangelist.h as detected
by lgtm scan.
Ensure `errflag` bitfield in struct flux_reactor is an unsigned
integer type. Detected by lgtm scan.
Ensure `blocked` bitfield in struct kvstx has unsigned type.
Detected by lgtm scan.
Remove unused imports from t/job-manager/*.py as detected by
lgtm scan.
@codecov-io
Copy link

Codecov Report

Merging #2605 into master will increase coverage by 0.02%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2605      +/-   ##
==========================================
+ Coverage   81.43%   81.45%   +0.02%     
==========================================
  Files         248      248              
  Lines       38495    38498       +3     
==========================================
+ Hits        31348    31359      +11     
+ Misses       7147     7139       -8
Impacted Files Coverage Δ
src/modules/kvs/kvstxn.c 76.72% <ø> (ø) ⬆️
src/common/libflux/reactor.c 92.18% <ø> (ø) ⬆️
src/shell/rc.c 87.56% <100%> (+0.09%) ⬆️
src/common/libsubprocess/local.c 80.2% <0%> (+0.34%) ⬆️
src/common/libsubprocess/server.c 71.54% <0%> (+0.56%) ⬆️
src/broker/modservice.c 72.18% <0%> (+0.75%) ⬆️
src/modules/job-info/guest_watch.c 75.85% <0%> (+1.13%) ⬆️

@mergify mergify bot merged commit e9a708b into flux-framework:master Jan 5, 2020
@grondo grondo deleted the lgtm branch January 5, 2020 22:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants