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 $filename and $mode types in python::dotfile #446

Merged
merged 3 commits into from
Nov 26, 2018

Conversation

gdubicki
Copy link

@gdubicki gdubicki commented Nov 4, 2018

erroneously changed in a990c0b

@bastelfreak bastelfreak changed the title Fix $filename type Fix $filename and $mode types in pyhton::dotfile Nov 4, 2018
@bastelfreak bastelfreak added the bug Something isn't working label Nov 4, 2018
@bastelfreak
Copy link
Member

Hi @gdubicki, thanks for pulling this out of the other PR. Can you please add a simple unit test to https://github.com/voxpupuli/puppet-python/tree/master/spec/defines that ensures that this now compiles?

.editorconfig Outdated Show resolved Hide resolved
@gdubicki gdubicki changed the title Fix $filename and $mode types in pyhton::dotfile Fix $filename and $mode types in python::dotfile Nov 4, 2018
@gdubicki
Copy link
Author

gdubicki commented Nov 4, 2018

Ready to review, @bastelfreak .

erroneously changed in a990c0b
and add tests to ensure this kind
of breakage will not repeat
@gdubicki
Copy link
Author

gdubicki commented Nov 5, 2018

Can you please trigger a rebuild in Travis, @danquack @bastelfreak ? I think it failed randomly.

@gdubicki
Copy link
Author

gdubicki commented Nov 7, 2018

It's failing on building image with Debian 9 and Puppet 6, long before testing my code. I propose to comment out this single setup from the matrix and re-enable it in a few days after it starts to work again (?) or fix it/keep it disabled if it wont (outside of this issue, of course).

as it fails long before running any code
test
@ekohl
Copy link
Member

ekohl commented Nov 8, 2018

Please don't comment those tests. They're temporary issues at Travis but still something we want to keep in. If it happens too often we should talk to Travis CI support about it.

@gdubicki
Copy link
Author

gdubicki commented Nov 8, 2018

This broken test is blocking my PR from being merged. And this PR is fixing completely broken dotfile resource.

So I suggest we keep my commenting out that test and you can create an issue to fix it and create PR in it to re-enable the test when its fixed, @ekohl.

@ekohl
Copy link
Member

ekohl commented Nov 8, 2018

It was just an intermittent failure. It was likely that a restart of that particular job would have passed. That's why I strongly feel this PR shouldn't modify it.

@bastelfreak
Copy link
Member

Thanks @gdubicki !

@bastelfreak bastelfreak merged commit ed0f134 into voxpupuli:master Nov 26, 2018
@danquack
Copy link
Contributor

@bastelfreak - I believe this PR fixed #447, so that issue can probably be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants