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.

Sign up for GitHub

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

Jump to bottom

Start managing CI files #269

Merged
merged 4 commits into from
Dec 13, 2019
Merged

Start managing CI files #269

merged 4 commits into from
Dec 13, 2019

Conversation

DavidS
Copy link
Contributor

@DavidS DavidS commented Dec 2, 2019

This requires a few changes to the pdk-templates from this PR: puppetlabs/pdk-templates#293

@DavidS DavidS requested a review from a team as a code owner December 2, 2019 11:58
@codecov-io
Copy link

codecov-io commented Dec 2, 2019 •
edited
Loading

Codecov Report

Merging #269 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #269   +/-   ##
=====================================
  Coverage       0%     0%           
=====================================
  Files           2      2           
  Lines         183    183           
=====================================
  Misses        183    183

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d9771bd...976d1ae. Read the comment docs.

glennsarti
glennsarti reviewed Dec 3, 2019
View reviewed changes
.travis.yml
@@ -1,22 +1,14 @@
---
os:
- linux
# OSX Only tests on the latest Puppet Gem, not the full matrix as there's no need to double up
Copy link
Contributor

Choose a reason for hiding this comment

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

CAn probably move this comment into .sync.yml

glennsarti
glennsarti approved these changes Dec 4, 2019
View reviewed changes
Copy link
Contributor

@glennsarti glennsarti left a comment

Choose a reason for hiding this comment

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

🚢 it!!!

Thanks @DavidS

glennsarti
glennsarti suggested changes Dec 4, 2019
View reviewed changes
Copy link
Contributor

@glennsarti glennsarti left a comment

Choose a reason for hiding this comment

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

And... I need to change my review

The Travis CI Test stage doesn't actually test anything. And rspec isn't running on MacOSX

@DavidS
Copy link
Contributor Author

DavidS commented Dec 4, 2019

@glennsarti I don't see how this PR would disable the testing - I'll have a more in-depth look later, but would appreciate any insight you have

@glennsarti
Copy link
Contributor

@DavidS It looks like the stages bit is "breaking" it. There's no CHECK environment variable.

@DavidS
Copy link
Contributor Author

DavidS commented Dec 4, 2019

@glennsarti that seems to be a problem unrelated to this PR. Looking at a previous run from https://travis-ci.org/puppetlabs/puppetlabs-powershell/builds/620050367 the exactly same behaviour is there. Might need a separate ticket then.

@glennsarti
Copy link
Contributor

@DavidS Ok so looks like #268 broke the testing 😢

IMO that needs to be rectified before this PR can be merged. However I'm not a primary maintainer so 🤷‍♂

@DavidS
Copy link
Contributor Author

DavidS commented Dec 5, 2019

Due to travis' propensity to multiply cells within a stage, I've now removed the main os key and instead put a singular osx cell into the acceptance stage (due to its duration). Now travis is testing on OS-X as initially planned.

We can revisit this when moving over to litmus.

@DavidS
Copy link
Contributor Author

DavidS commented Dec 11, 2019

@glennsarti are you happy with this solution?

@DavidS DavidS merged commit 7d5c681 into master Dec 13, 2019
@DavidS DavidS deleted the start_managing_ci branch December 13, 2019 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@glennsarti glennsarti Awaiting requested review from glennsarti

Assignees
No one assigned
Labels
maintenance
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@DavidS @codecov-io @glennsarti