search

dev-sec / cis-dil-benchmark

CIS Distribution Independent Linux Benchmark - InSpec Profile
Apache License 2.0
149 stars 92 forks source link

GitHub action #103

Closed rndmh3ro closed 3 years ago

deric4 commented 3 years ago

@rndmh3ro @schurzi is there any more context on this PR somewhere? This is sort of a breaking change isn't it since none of the test actions are passing? The rubocop changes were discussed briefly in #90 .

rndmh3ro commented 3 years ago

We changed the travis tests with github actions. There was only some internal discussion. We plan to fix the tests, too.

Why do you think this is a breaking change? The code itself did not change.

schurzi commented 3 years ago

my rationale for merging this was, that it may be not a great idea to change the complete CI workflow and do a lot of syntactic code changes in one PR. We are already working on fixng the lint recommendations. :)

deric4 commented 3 years ago

Why do you think this is a breaking change? The code itself did not change.

Prior to this change the convention seemed to be that all the Checks must pass (DCO and Travis). Now that travis has been removed, there are 3 failing Checks now for the most current release:

https://github.com/dev-sec/cis-dil-benchmark/actions/runs/511972343

which i believe are caused by the .rubocop.yml changes, some of which were allowed in order to align with the Inspec Style Guide recommendations

my rationale for merging this was, that it may be not a great idea to change the complete CI workflow and do a lot of syntactic code changes in one PR. We are already working on fixng the lint recommendations. :)

@schurzi , what will be the expected workflow for accepting PRs in the interim?

rndmh3ro commented 3 years ago

Prior to this change the convention seemed to be that all the Checks must pass (DCO and Travis). Now that travis has been removed, there are 3 failing Checks now for the most current release:

This is not a breaking change according to semantic versioning. For the baseline itself this is not a change at all since we didn't change any of the code.

deric4 commented 3 years ago

This is not a breaking change according to semantic versioning. For the baseline itself this is not a change at all since we didn't change any of the code.

Valid point, my concern was more with how future PRs will be handled. Previously the convention was that both the DCO and Lint checks must pass before the PR was approved/merged. Would it be possible to revert the rubocop changes until the syntax updates described previously are ready so as not to cause failures in PRs in the mean time?

rndmh3ro commented 3 years ago

Valid point, my concern was more with how future PRs will be handled

Yeah, you're right. This wasn't the best move, sorry. I'll fix this tomorrow. Either by fixing the linting or reverting.

deric4 commented 3 years ago

@rndmh3ro thanks! also very excited to see more use of GitHub Actions!

schurzi commented 3 years ago

@deric4 sorry for the hassle. I already prepared a PR to get this in order again (#104)