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

Add coala support and fix/lint files #121

Merged
merged 3 commits into from
Jan 4, 2017

Conversation

umeshksingla
Copy link
Contributor

No description provided.

@@ -0,0 +1,41 @@
[Default]
ignore = .tox/**, .gitignore, .gitreview, .gitmodules, **/provisioning/roles/**
Copy link
Owner

Choose a reason for hiding this comment

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

.gitreview, .gitmodules, /provisioning/roles/

Don't apply to this repo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Working on it. Sorry, shouldn't have blindly copied the coafile. One thing, is it fine to keep .tox/** and the check on .sh files?

Copy link
Owner

@dfarrell07 dfarrell07 Dec 20, 2016

Choose a reason for hiding this comment

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

There aren't any .sh files in this repo (find . -name "*.sh" ) and thinking about Puppet I can't imagine we'll end up writing shell helpers or something, so I guess it can be removed.

Sorry, shouldn't have blindly copied the coafile.

np, just include "WIP" (work in progress) in the commit message if you're planning on updating the commit

Copy link
Owner

Choose a reason for hiding this comment

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

Ah, missed tox question. Yes, I guess we can remove that too, we use Rakefile for that job in puppet

Copy link
Contributor

@srisankethu srisankethu Dec 20, 2016

Choose a reason for hiding this comment

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

coala has PuppetLintBear. We can add that for files = **/*.pp. I am currently writing a bear for Ansible which should be updated in the new release.

Copy link
Contributor Author

@umeshksingla umeshksingla Dec 20, 2016

Choose a reason for hiding this comment

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

I added bear for puppet. For now, I'm ignoring class-name related and some line-length warnings by puppet bear.

And for Ruby, what Bear should we go for RuboCopBear or RubySyntaxBear? RuboCopBear gives a lot of warnings, a lot like "Method/class has too many lines", line-length, using Ruby 1.9 hash syntax, use of braces etc while RubySyntaxBear just checks for the syntax and has few (5-6) warnings. We can go with the first one, ignore all the warnings for now and follow it in future. @dfarrell07 what do you suggest?

Copy link
Owner

Choose a reason for hiding this comment

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

@umeshksingla The more linters the better? I don't have a strong preference, whatever you think's best. :)

ignore = .tox/**, .gitignore, .gitreview, .gitmodules, **/provisioning/roles/**
use_spaces = True

[DOCS]
Copy link
Owner

Choose a reason for hiding this comment

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

Doesn't apply

files = ./**/*.markdown, ./**/*.md
default_actions = MarkdownBear: ApplyPatchAction

[dockerfile]
Copy link
Owner

Choose a reason for hiding this comment

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

Doesn't apply

bears = ShellCheckBear
files = ./**/*.sh

[python]
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think this applies

files = ./**/*.py
default_actions = PyUnusedCodeBear: ApplyPatchAction

[autopep8]
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think this applies

@@ -18,17 +18,17 @@ env:
- PUPPET_VERSION="~> 4.3.0"
matrix:
exclude:
- rvm: 2.2.3
Copy link
Owner

Choose a reason for hiding this comment

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

All of these conflict with #120

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This changes only the indentation, should I ignore this file for these changes for now?

Copy link
Owner

Choose a reason for hiding this comment

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

#120 has been merged so maybe you can rebase to include those changes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, on it.

Copy link
Owner

@dfarrell07 dfarrell07 left a comment

Choose a reason for hiding this comment

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

Should likely review the file types in this repo to see if there are new sections to add to .coafile (Ruby comes to mind)

@umeshksingla umeshksingla changed the title Add coala support and fix/lint files WIP: Add coala support and fix/lint files Dec 20, 2016
Additional dependencies - puppet-lint, xmllint, rubocop

Only very obvious changes have been made, ignoring
class-name, line-length, string-literal related warnings

Signed-off-by: Umesh Singla <[email protected]>
@dfarrell07
Copy link
Owner

Needs to rebase to include #120, see if the tests pass after

@umeshksingla
Copy link
Contributor Author

All checks have passed now, so you can merge, I suppose.

@dfarrell07
Copy link
Owner

Looks great, thanks! 👍

@dfarrell07 dfarrell07 merged commit e2a26e2 into dfarrell07:master Jan 4, 2017
@umeshksingla umeshksingla changed the title WIP: Add coala support and fix/lint files Add coala support and fix/lint files Jan 4, 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.

3 participants