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

AO3-6549 Remove ignore of auto collection invite column #4568

Merged
merged 4 commits into from
Aug 13, 2023

Conversation

brianjaustin
Copy link
Member

Pull Request Checklist

Issue

https://otwarchive.atlassian.net/browse/AO3-6549

Purpose

Remove the ignore for the column that was deleted in #4505

Testing Instructions

Reference Jira

References

Are there other relevant issues/pull requests/mailing list discussions?

Credit

Brian Austin (they/he)

@brianjaustin
Copy link
Member Author

Since the change was so small, I also went through the RuboCop issues for that file (and added tests). They should be addressed eventually either way, but if we prefer not to do all this at once I can revert some or all of them

app/models/preference.rb Fixed Show fixed Hide fixed
Copy link
Contributor

@ceithir ceithir left a comment

Choose a reason for hiding this comment

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

I fear you've opened Pandora's box: Now you have to update a regexp to a new syntax, and so must add a bunch more tests to make quite sure nothing was broken in the process.

What's currently in the PR seems good though. 👍

@brianjaustin
Copy link
Member Author

@ceithir thanks for the review! Based on https://stackoverflow.com/a/3632051, I think it won't actually be too bad (just the extra test which would fail with the old regex)

spec/models/preference_spec.rb Outdated Show resolved Hide resolved
spec/models/preference_spec.rb Outdated Show resolved Hide resolved
@sarken sarken merged commit 98b0c2e into otwcode:master Aug 13, 2023
23 checks passed
@brianjaustin brianjaustin deleted the AO3-6549-remove-ignore branch August 19, 2023 23:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants