Skip to content

Conversation

@boegel
Copy link
Member

@boegel boegel commented Mar 10, 2017

@wpoely86 While working on additional style check for the order of easyconfig parameters, I noticed that we're not using the checker_state that pep8 supports, which comes in useful in the check for trailing whitespace where we want to ignore trailing whitespace in the description...

@boegel boegel added this to the 3.1.2 milestone Mar 10, 2017
# keep track of name of last parameter that was defined
param_def = PARAM_DEF_REGEX.search(line)
if param_def:
checker_state['eb_last_key'] = param_def.group('key')
Copy link
Member

Choose a reason for hiding this comment

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

this is nice!

comment_re = re.compile(r'^\s*#')
if comment_re.match(physical_line):
# apparently this is not the same as physical_line line?!
line = lines[line_number-1]
Copy link
Member

Choose a reason for hiding this comment

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

why switch?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because apparently this is not the same as physical_line... I noticed that for the description = line, only the part after the = was included in physical_line...

({}, None),
({'eb_last_key': 'description'}, None),
({'eb_last_key': 'description'}, None),
({'eb_last_key': 'description'}, (21, "W299 trailing whitespace")),
Copy link
Member Author

Choose a reason for hiding this comment

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

I should change this, I should be specifying the state manually, but just use state = {} before the loop and then pass in state without touching it in the test.

@boegel
Copy link
Member Author

boegel commented Mar 11, 2017

Thanks for the review @wpoely86!

@boegel boegel merged commit 729b980 into easybuilders:develop Mar 11, 2017
@boegel boegel deleted the style_check_state_test branch March 11, 2017 17:04
boegel added a commit to boegel/easybuild-framework that referenced this pull request Mar 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants