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

docs: Switch to Ethical Ads #18090

Merged
merged 4 commits into from Feb 9, 2024
Merged

docs: Switch to Ethical Ads #18090

merged 4 commits into from Feb 9, 2024

Conversation

harish-sethuraman
Copy link
Member

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

[x] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)

Is there anything you'd like reviewers to focus on?

nope

@harish-sethuraman harish-sethuraman requested a review from a team as a code owner February 7, 2024 18:49
@eslint-github-bot eslint-github-bot bot added the feature This change adds a new feature to ESLint label Feb 7, 2024
Copy link

netlify bot commented Feb 7, 2024

Deploy Preview for docs-eslint ready!

Name Link
🔨 Latest commit 3ab24f9
🔍 Latest deploy log https://app.netlify.com/sites/docs-eslint/deploys/65c59624aa5d7c00083ee603
😎 Deploy Preview https://deploy-preview-18090--docs-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@harish-sethuraman harish-sethuraman marked this pull request as draft February 7, 2024 18:49
@harish-sethuraman harish-sethuraman added the documentation Relates to ESLint's documentation label Feb 7, 2024
@harish-sethuraman harish-sethuraman changed the title feat: Switch to Ethical Ads docs: Switch to Ethical Ads Feb 7, 2024
@harish-sethuraman harish-sethuraman removed the feature This change adds a new feature to ESLint label Feb 7, 2024
@@ -4,6 +4,15 @@
}
}

.docs-ad {
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 had a wrapper in the main page to avoid shift of content when ad pops up. Seems like docs didn't have it so added this.

docs/src/_data/sites/en.yml Outdated Show resolved Hide resolved
@nzakas nzakas marked this pull request as ready for review February 8, 2024 22:03
@nzakas
Copy link
Member

nzakas commented Feb 8, 2024

We are ready to roll this out pending the removal of the unnecessary comments.

@nzakas
Copy link
Member

nzakas commented Feb 8, 2024

Once merged, we should also merge this: eslint/eslint.org#522

@harish-sethuraman harish-sethuraman self-assigned this Feb 9, 2024
@mdjermanovic mdjermanovic added the accepted There is consensus among the team that this change meets the criteria for inclusion label Feb 9, 2024
Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

I'm leaving this open if @nzakas wants to take another look before merging, and because ideally this should be merged just before we start today's release so that the docs sites are updated around the same time (eslint.org/docs/head right before and eslint.org/docs/next right after the release).

What about eslint.org/docs/latest? Looks like we should prepare another PR that should target the latest branch?

@mdjermanovic
Copy link
Member

What about eslint.org/docs/latest? Looks like we should prepare another PR that should target the latest branch?

#18102

Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

LGTM. I'll let @mdjermanovic merge just before the release.

@mdjermanovic
Copy link
Member

Since we've already merged #18102 and the new ads are live on eslint.org/docs/latest, we could as well merge this one now? :)

@nzakas
Copy link
Member

nzakas commented Feb 9, 2024

Oops, yes. :)

@nzakas nzakas merged commit f1c7e6f into main Feb 9, 2024
18 checks passed
@nzakas nzakas deleted the chore/add-ads branch February 9, 2024 17:39
@mdjermanovic mdjermanovic mentioned this pull request Feb 13, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion documentation Relates to ESLint's documentation
Projects
Status: Complete
Development

Successfully merging this pull request may close these issues.

None yet

3 participants