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.

Sign up for GitHub

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

Jump to bottom

Run Prettier on JS code fences, part 6 #21175

Closed
wants to merge 2 commits into from
Closed

Run Prettier on JS code fences, part 6 #21175

wants to merge 2 commits into from

Conversation

OnkarRuikar
Copy link
Contributor

@OnkarRuikar OnkarRuikar commented Sep 29, 2022 •
edited
Loading

The PR focuses only on JS code fences.

Idea is to gradually prettify all the JS code fences before the full automation.

cc/ @Josh-Cena

@OnkarRuikar
Run Prettier on JS code fences, part 6
55cb836 
The PR focuses only on JS code fences.

Idea is to gradually prettify all the JS code fences before the full automation.
@OnkarRuikar OnkarRuikar requested a review from a team as a code owner September 29, 2022 06:22
@OnkarRuikar OnkarRuikar requested review from rebloor and removed request for a team September 29, 2022 06:22
@github-actions github-actions bot added Content:Other Any docs not covered by another "Content:" label Content:WebExt WebExtensions docs labels Sep 29, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Sep 29, 2022 •
edited
Loading

Preview URLs (66 pages)
Flaws (1)

Note! 65 documents with no flaws that don't need to be listed. 🎉

URL: /en-US/docs/Mozilla/Add-ons/WebExtensions/API/devtools/panels/ExtensionSidebarPane/setPage
Title: devtools.panels.ExtensionSidebarPane.setPage()
Flaw count: 1

  • bad_bcd_queries:
    • No BCD data for query: webextensions.api.devtools.panels.ExtensionSidebarPane.setPage
External URLs (4)

URL: /en-US/docs/Mozilla/Add-ons/WebExtensions/API/devtools/panels/ExtensionSidebarPane/onShown
Title: devtools.panels.ExtensionSidebarPane.onShown

URL: /en-US/docs/Mozilla/Add-ons/WebExtensions/API/devtools/panels/ExtensionSidebarPane/setExpression
Title: devtools.panels.ElementsPanel.setExpression()

URL: /en-US/docs/Mozilla/Add-ons/WebExtensions/API/devtools/panels/ExtensionSidebarPane/onHidden
Title: devtools.panels.ExtensionSidebarPane.onHidden

(this comment was updated 2022-11-07 13:20:57.003288)

@wbamberg
Copy link
Collaborator

Please see #20280 (comment). @Rob--W , what is the status of your internal discussions? I'm not sure what is the best way forward if the WebExtensions team doesn't want Prettier.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 7, 2022

This pull request has merge conflicts that must be resolved before it can be merged.

@Rob--W
Copy link
Member

Rob--W commented Nov 11, 2022

Please see #20280 (comment). @Rob--W , what is the status of your internal discussions? I'm not sure what is the best way forward if the WebExtensions team doesn't want Prettier.

Sorry for missing this comment.

The team agreed with the concerns expressed in #20280 (comment)

Doesn't necessarily mean that we want to keep everything unchanged. There were additional remarks: one of my expressed concerns was over the readability of .then. Two team members wondered why we didn't use await there instead. That could potentially improve the readability of the code and make Prettier happy.

A new team member who went through the onboarding with help of MDN particularly agreed with this point that I raised:

The second class of code blocks are examples. In this case, simplicity is important to novice readers. Having a separate variable name can make it easier to read code.

I fully recognize the value of a uniform formatting across the documentation. That should not be at the expense of the ease of understanding. If we can configure Prettier to achieve uniformity and not sacrifice comprehensibility, then we are good to proceed.

The readability of code is subjective. What is not subjective is the (in)ability to Ctrl-F when the reference to an API is split over multiple lines (see the browser.bookmarks.create example at #20280 (comment)). Not splitting the namespace+method name is therefore a hard requirement for the extension documentation.

@Josh-Cena
Copy link
Member

Since the WebExt section more or less has its own governance, it's best if the WebExt team can make decisions about their own coding style. As long as they believe the current state is okay, it's fine if we don't apply prettier here at this stage.

@Josh-Cena Josh-Cena closed this Dec 31, 2022
@Josh-Cena Josh-Cena mentioned this pull request Feb 1, 2023
Merged
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 1, 2024
@OnkarRuikar OnkarRuikar deleted the js-code-lint7 branch July 31, 2024 02:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@rebloor rebloor Awaiting requested review from rebloor rebloor is a code owner automatically assigned from mdn/yari-content-mozilla-add-ons

Assignees
No one assigned
Labels
Content:Other Any docs not covered by another "Content:" label Content:WebExt WebExtensions docs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@OnkarRuikar @wbamberg @Rob--W @Josh-Cena