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

Restore clean-header-search-field for some users #6311

Merged
merged 1 commit into from
Feb 11, 2023
Merged

Conversation

fregante
Copy link
Member

@fregante fregante commented Feb 8, 2023

@fregante fregante added the bug label Feb 8, 2023
@kaste
Copy link

kaste commented Feb 8, 2023

Works for me!

const headerSearchButton = await elementReady('button.header-search-button, input.header-search-input');
if (headerSearchButton instanceof HTMLInputElement) {
// Previous version #6310
// TODO: Remove in mid 2023
Copy link
Member

Choose a reason for hiding this comment

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

Maybe put a real date so it shows up as a warning

Copy link
Member Author

Choose a reason for hiding this comment

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

I know we've been doing that, but what does that change in practice? We don't want CI to start failing and force us to immediately take action 🤔

Copy link
Member

Choose a reason for hiding this comment

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

If I am not mistaken we set it to warn

Copy link
Member Author

Choose a reason for hiding this comment

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

We did, but due to:

I think expiring-todo-comments only makes sense:

  • if it never breaks (on a date) and forces anyone to act immediately
  • if someone is actively looking for and fixing TODO comments
  • if there are no other TODOs that are also waiting for a fix anyway, for months

Essentially I think we can do away with them. Moving this discussion upstream in sindresorhus/eslint-plugin-unicorn#1541

@auscompgeek
Copy link

Untested, depends on the user

It should be toggleable in the feature preview settings:

image

@fregante fregante merged commit 4132e12 into main Feb 11, 2023
@fregante fregante deleted the clean-header branch February 11, 2023 05:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

clean-header-search-field: Cannot read properties of undefined (reading 'classList')
4 participants