-
Notifications
You must be signed in to change notification settings - Fork 71
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 a job to warn about modulesync changes #727
base: master
Are you sure you want to change the base?
Conversation
Is there a way we can do this for one module and see what it looks like when you edit a file you shouldn't have? |
okay so this is really awesome! |
That was on my agenda: voxpupuli/puppet-example#8. There it really shows this was 100% just a thought because the Github actions syntax is invalid :) I'll continue to iterate there. |
01cfc20 added headers, but this isn't really a good experience for users. It's actually much better to fail in CI if a diff is detected. That gives them also a way to verify their changes to .sync.yml are correct. This takes the approach of checking out the modulesync config with the version from .msync.yml and running a one off change in offline mode. Then it uses git diff to see if there are differences. That should fail the build and also show the actual differences, which makes debugging easier.
bf2c78e
to
03c07b1
Compare
I think I've worked out all the kinks in voxpupuli/puppet-example#8 and this is now ready for review. |
working-directory: msync_config | ||
|
||
- name: Check for differences | ||
run: git diff --exit-code |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we need to run git add . && git diff --cached --exit-code
to also track added/removed files
ruby-version: '3.0' | ||
|
||
- name: Set modulesync version | ||
run: ruby -ryaml -e 'puts "MSYNC_VER=#{YAML.safe_load(File.read(".msync.yml"))["modulesync_config_version"]}"' >> $GITHUB_ENV |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would break with pre-release like in voxpupuli/puppet-telegraf#174. Perhaps we need to update our .msync.yml
file to write git describe
there instead of hardcoding.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the question is do we want to support prereleases, or do we want to be more strict with modulesync_config releases? I'm not sure here.
01cfc20 added headers, but this isn't really a good experience for users. It's actually much better to fail in CI if a diff is detected. That gives them also a way to verify their changes to .sync.yml are correct.
This takes the approach of checking out the modulesync config with the version from .msync.yml and running a one off change in offline mode. Then it uses git diff to see if there are differences. That should fail the build and also show the actual differences, which makes debugging easier.
Right now this is all theoretical and the principe should work, but isn't tested. I wanted to share it early on to open a discussion.
Another thing to think about is to make this easy for individual developers on their workstations. Something like a PDK mode for modulesync.