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

Fix commented out cops due to renames #981

Merged
merged 14 commits into from
Apr 1, 2025
Merged

Conversation

tas50
Copy link
Contributor

@tas50 tas50 commented Dec 1, 2024

Description

Fix the cops broken during one of my upgrades a long time ago. This is probably going to result in CI failures elsewhere as these cops have been disabled for a while. Test Kitchen triggered on one of the alignment cops.

Related Issue

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Chore (non-breaking change that does not add functionality or fix an issue)

Checklist:

  • I have read the CONTRIBUTING document.
  • I have run the pre-merge tests locally and they pass.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • If Gemfile.lock has changed, I have used --conservative to do it and included the full output in the Description above.
  • All new and existing tests passed.
  • All commits have been signed-off for the Developer Certificate of Origin.

@sean-simmons-progress sean-simmons-progress added the Triage: Needs Information Indicates an issue needs more information in order to work on it. label Dec 10, 2024
@dafyddcrosby
Copy link
Collaborator

Quick $0.02 for Progress reviewers - a potential sticking point is specifically the cookstyle.yml changes, which as Tim pointed out will fire on downstream CI changes. Some discussion @tpowell-progress and I had (and discussed in the CAC around the time of moving Chefstyle into Cookstyle) was 'streams' (#967) to avoid that issue, but I haven't had time to get that sorted out yet. I'm personally fine with more lints enabled, but I'm not a typical customer, either ;-)

tas50 added 11 commits February 4, 2025 13:39
These were renamed in rubocop/rubocop#7460

Signed-off-by: Tim Smith <[email protected]>
Signed-off-by: Tim Smith <[email protected]>
This one was partially updated at some point, but is not right:

See https://github.com/rubocop/rubocop/blob/e12b51389500da8e744730f8fa022d739ba0b573/config/obsoletion.yml#L45

Signed-off-by: Tim Smith <[email protected]>
Signed-off-by: Tim Smith <[email protected]>
@tas50 tas50 force-pushed the tas50/fix_renames branch from 1501126 to 147102a Compare February 4, 2025 21:39
@tas50
Copy link
Contributor Author

tas50 commented Mar 15, 2025

Checking in here 3 months later. Any chance Progress is ready to merge this now?

Signed-off-by: Tim Smith <[email protected]>
@tas50 tas50 force-pushed the tas50/fix_renames branch from c81ee9e to c2bbb73 Compare March 15, 2025 03:43
@jaymzh
Copy link
Collaborator

jaymzh commented Mar 18, 2025

Hey @tas50 - we'd like do a first-pass with these rules on at least the top-few chef repos (chef/chef, chef/ohai mixlib/*) so that contributors don't run into errors unrelated to their change.

Would you be willing to do that, and then we can merge?

If you don't have the cycles, @dafyddcrosby (community member) is willing to do it, but may not have time until May.

tas50 added 2 commits March 26, 2025 08:52
New rubocop-performance releases require more modern rubocop gems

Signed-off-by: Tim Smith <[email protected]>
Once we're on a new Rubocop release this can go away

Signed-off-by: Tim Smith <[email protected]>
tas50 added a commit to tas50/chef-workstation that referenced this pull request Mar 26, 2025
This PR was requested in chef/cookstyle#981

Signed-off-by: Tim Smith <[email protected]>
@tas50
Copy link
Contributor Author

tas50 commented Mar 27, 2025

@jaymzh Looks like a TON of those projects are still using chefstyle, so someone internally is going to need to do all the work to get things updated to cookstyle and linted with these new rules. That's a bit too much of a lift for me given all the other job work I have.

@tas50 tas50 mentioned this pull request Mar 29, 2025
11 tasks
tas50 added a commit to tas50/chef that referenced this pull request Mar 30, 2025
This gets things passing with chef/cookstyle#981

Signed-off-by: Tim Smith <[email protected]>
@Vasu1105 Vasu1105 merged commit f9ecbdf into chef:main Apr 1, 2025
8 checks passed
Copy link
Contributor

@Vasu1105 Vasu1105 left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Triage: Needs Information Indicates an issue needs more information in order to work on it.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants