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

Tidy and Critic should not check pod files #119

Merged
merged 3 commits into from
Jan 7, 2017

Conversation

stdweird
Copy link
Member

No description provided.

@stdweird stdweird added this to the 16.12 milestone Dec 15, 2016
@stdweird stdweird changed the title Tidy and Critic shoul not check pod files Tidy and Critic should not check pod files Dec 15, 2016
@stdweird
Copy link
Member Author

@jrha ideally, this gets merge and a release is made (for maven; making an rpm as part of the release is not needed atm). i'm testing ncm-ncd atm, it's very useful

Copy link
Contributor

@ned21 ned21 left a comment

Choose a reason for hiding this comment

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

I don't understand the point of 4226eec but the code otherwise looks OK

@stdweird
Copy link
Member Author

@ned21 we have quite a bit code that does not use Test::Quattor to do actual non-mocked testing (in particular, CAF can't use it). but it should always be non-disruptive to import Test::Quattor::Object in existing tests to enable the warning test.

@ned21
Copy link
Contributor

ned21 commented Dec 16, 2016

@stdweird thanks for the explanation. Apologies I actually seem to have copy & pasted the wrong commit id. It was 85c9e66 (critic and tidy codedirs default to doc poddirs) that I didn't understand.

@stdweird
Copy link
Member Author

as we have most of the pod either in the scripts and/or modules, or have pod file in the directory as the code; the checks typically scan the same directories for code to check. so instead of configuring it 3 times with the same value, we can do so using one value. the doc directory is chosne because we currently have pod tests with the list of dirs.

eg https://github.com/quattor/maven-tools/blob/master/build-scripts/src/test/perl/00-tqu.t#L12
i should remove the codedirs value in the critic and tidy section.

@ned21
Copy link
Contributor

ned21 commented Dec 17, 2016

OK. Since 16.12 is done and @jrha is on holiday, should this get merged or wait for @jrha ?

@stdweird
Copy link
Member Author

@ned21 i don't think 16.12 is out, but maven-tools doesn't follow the releases anyway
i also pushed and squashed a commit fixing the redundant codedir entries in the 00-tqu unittest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants