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

Add a job to warn about modulesync changes #727

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

Conversation

ekohl
Copy link
Member

@ekohl ekohl commented Jul 13, 2021

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.

@ghoneycutt
Copy link
Member

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?

@bastelfreak
Copy link
Member

okay so this is really awesome!

@ekohl
Copy link
Member Author

ekohl commented Jul 14, 2021

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?

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.
@ekohl
Copy link
Member Author

ekohl commented Jul 14, 2021

I think I've worked out all the kinks in voxpupuli/puppet-example#8 and this is now ready for review.

@ekohl ekohl marked this pull request as ready for review July 14, 2021 12:37
working-directory: msync_config

- name: Check for differences
run: git diff --exit-code
Copy link
Member Author

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
Copy link
Member Author

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.

Copy link
Member

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.

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