Skip to content

Comments

(PUP-5545) rubocop by default#4463

Closed
ghoneycutt wants to merge 2 commits intopuppetlabs:masterfrom
ghoneycutt:PUP-5545-rubocop_by_default
Closed

(PUP-5545) rubocop by default#4463
ghoneycutt wants to merge 2 commits intopuppetlabs:masterfrom
ghoneycutt:PUP-5545-rubocop_by_default

Conversation

@ghoneycutt
Copy link
Contributor

No description provided.

@HAIL9000
Copy link
Contributor

Hey @ghoneycutt, it looks like the Rubocop check is now failing in Travis, could you take a quick look and see what's going on there?

@HAIL9000 HAIL9000 changed the title Pup 5545 rubocop by default (PUP-5545) rubocop by default Nov 30, 2015
@ghoneycutt
Copy link
Contributor Author

Hi @HAIL9000

Seems that the rubocop testing for puppet itself is using an older version and was attempting to read the .rubocop.yml file in the skeleton. I added a commit which tells the rubocop for the puppet project to ignore files in the puppet skeleton itself.

https://github.com/puppetlabs/puppet/pull/4463/files#diff-11a0d7906801d9dea0eccb85667ad811R15

@ghoneycutt
Copy link
Contributor Author

Don't merge yet.. I found some more additions to the configuration while testing another module that contained custom facts. I'll also need to run this over a module with custom types and providers to ensure it isn't suggesting changes for things that are common place in the community.

@kylog
Copy link

kylog commented Jan 6, 2016

Ping @andersonmills

Copy link
Contributor

Choose a reason for hiding this comment

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

@hunner Is this the style y'all use?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we have a specified format yet, but voxpupuli has prior-art on this and it appears to be the same https://github.com/voxpupuli/modulesync_config/blob/master/moduleroot/.rubocop.yml 👍

@MikaelSmith
Copy link
Contributor

Needs a rebase :(

@HAIL9000
Copy link
Contributor

HAIL9000 commented Feb 9, 2016

Hey @ghoneycutt, do you have some time to give this a quick rebase? Looks like there are merge conflicts

@joshcooper
Copy link
Contributor

Hey @ghoneycutt could you rebase? @andersonmills can you follow up and either merge when the changes become available or close if there is no activity?

@puppetcla
Copy link

CLA signed by all contributors.

@ghoneycutt ghoneycutt force-pushed the PUP-5545-rubocop_by_default branch from 8565e71 to ef014bd Compare April 7, 2016 17:04
@ghoneycutt
Copy link
Contributor Author

@joshcooper @HAIL9000

Rebased

@MikaelSmith
Copy link
Contributor

I've opened a rebased PR at #4936 for @andersonmills to test. If it looks good we'll merge it, otherwise might have more requests for work.

@MikaelSmith MikaelSmith closed this May 4, 2016
@andersonmills
Copy link
Contributor

Closed in deference to #4936

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.

9 participants