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

Refactor: Switch out ~ expression in tag for not statement #935

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

luke-hill
Copy link
Contributor

Summary

Switch out legacy tag expression syntax - This is needed to fix up some tech debt in cucumber (Which sanitizes it)

Details

As written above

Motivation and Context

How Has This Been Tested?

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 not work as expected)
  • Documentation update
  • Internal change (refactoring, test improvements, developer experience or update of dependencies)

Checklist:

  • I've added tests for my code
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@luke-hill
Copy link
Contributor Author

luke-hill commented Jul 10, 2024

Hi @mvz
I'm not sure what's going on here with the CI e.t.c. so I've merged this as it's a tiny bugfix. I'll let you decide if anything needs remedying after the fact as I don't quite know where the CI is in this repo (I notice lots of the PR's are ❌ too)

EDIT: Somethings up with the project structure and I can't. So I figure someone else will need to.

@mvz
Copy link
Contributor

mvz commented Jul 10, 2024

@luke-hill one thing that makes the PR fail is the change in quote characters in lib/aruba/cucumber/hooks.rb. Did that happen automatically due to some some editor setting?

Perhaps I need to adjust the RuboCop config so it matches up with the rest of Cucumber. Suggestions welcome!

@luke-hill
Copy link
Contributor Author

In terms of styling. We use single quotes in all ruby projects. Also we don't adhere to the gemspec rule about dev dependencies in the gemfile (No idea why that's a default).

I can amend the quotes. I think I clicked something in my editor for that, as I didn't use rubocop on the CLI. The other one I've no idea on though

@mvz
Copy link
Contributor

mvz commented Jul 11, 2024

@luke-hill allright, here's my plan:

  • I fix the complaint from rubocop about add_runtime_dependency
  • I update Aruba's rubocop config to be (more) in line with the ruby cucumber gems and autocorrect the code to match it
  • You rebase this pull request, and the rubocop complaints should go away
  • I merge the pull request

@mvz
Copy link
Contributor

mvz commented Oct 20, 2024

Hi @luke-hill, I have finally done the second step in my plan above. Could you please rebase your branch on current main?

@luke-hill
Copy link
Contributor Author

Will do.

It's also made me think we should have an overall cucumber style for rubocop as a separate gem that way it saves the hassle of setting things up each time. But I'm not sure when I'll get around to it.

@mvz
Copy link
Contributor

mvz commented Oct 28, 2024

It's also made me think we should have an overall cucumber style for rubocop as a separate gem that way it saves the hassle of setting things up each time.

That's a good idea and will help a lot to make the settings more consistent.

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.

2 participants