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

PDK v2.5.0 update #162

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

PDK v2.5.0 update #162

wants to merge 1 commit into from

Conversation

jeffbyrnes
Copy link

@jeffbyrnes jeffbyrnes commented Aug 20, 2020

SUMMARY

  • Automatically updated via pdk update using PDK v2.5.0
  • Clean up Ruby syntax per rubocop

TESTS/SPECS

  • Ran pdk validate, works as expected
  • Ran pdk test unit, works as expected

@jeffbyrnes jeffbyrnes marked this pull request as draft August 20, 2020 22:41
@jeffbyrnes jeffbyrnes marked this pull request as ready for review August 24, 2020 18:00
@jeffbyrnes
Copy link
Author

@jsok some maintenance updates here; some of the gems I’m keeping might not be necessary anymore, but I lack enough knowledge of how they may have been incorporated into puppet-rspec generally, if at all.

@vStone
Copy link

vStone commented Sep 8, 2020

I did somewhat the same thing but added a .sync.yml file to make it future proof:

---
Gemfile:
  optional:
    ':development':
      - gem: rspec-json_expectations

spec/spec_helper.rb:
  mock_with: ':rspec'
  spec_overrides: |
    require 'rspec/json_expectations'

This will make sure the Gemfile and spec_helper.rb are automatically updated too.

@jeffbyrnes
Copy link
Author

@vStone nice! I’ll add that to this PR.

Copy link
Owner

@jsok jsok left a comment

Choose a reason for hiding this comment

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

There's a lot of changes in this PR, would it be possible to break it down into a few smaller ones?
e.g. there's some purely linting related changes, others about spec setup and updates, then there's some drastic CI changes along with what seems to be many file unrelated to this project (probably things specific to your local dev environment).

TBH I'm not on top of PDK so any explanations in the PR description would be a big help too.

I'll be able to review and approve the changes quicker with this approach.

@jeffbyrnes
Copy link
Author

jeffbyrnes commented Sep 28, 2020

@jsok so these all came to be as a result of running pdk update, so my impression is that they’re meant to be done altogether.

That said, looking at it freshly after being away from it for a bit, there’s some things here that clearly shouldn’t be changed. So lemme see what I can do.

@jeffbyrnes jeffbyrnes changed the title tests: pdk v1.18.1 update PDK v2.5.0 update Jul 28, 2022
@jeffbyrnes
Copy link
Author

Re-ran this with PDK v2.5.0, and rebased this.

As for explanations, PDK has a concept of “all modules should include these elements for consistency” and pdk update is how you can bring an existing module up-to-snuff with these conventions.

jeffbyrnes added a commit to athenahealth/puppet-vault that referenced this pull request Oct 12, 2022
* Automatically update via pdk update using PDK v2.5.0
* Add `.sync.yml` per @vStone in jsok#162 to future-proof
* Automatically update via pdk update using PDK v2.5.0
* Add `.sync.yml` per @vStone in jsok#162 to future-proof
@jeffbyrnes
Copy link
Author

Updated this yet again, and also added @vStone’s suggestion of a .sync.yml.

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