-
Notifications
You must be signed in to change notification settings - Fork 499
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
Triage all KumaScript macros that are called from our JS docs #2846
Comments
This list of macros uses mdn/stumptown-content#264 to lint for all macro calls under /en-US/docs/Web/JavaScript:
|
This comment presents an initial triage for the 33 macros listed above and called from our JS docs. It should assign to each macro one of the following three options:
For macros we intend to replace, there's going to be a further choice about whether we expose them to guide pages via "directives" (as we already do for BCD, for example), or restrict them to pages that are of a particular recipe (as we do for specifications, for example).
Replace We're intending to replace sidebars, of course.
Render These are all XRef macros, that convert some arguments (a partial path, in effect, plus sometimes some extra arguments) to a link.
Replace These are used to add specially styled "Next" and "Previous" links to documents that are part of a linear sequence. We don't yet have an implementation of this in stumptown, but see mdn/stumptown-content#265 for a proposal.
Replace This is an "info box" for JS properties. See e.g. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/length. We know we will want info box support in stumptown (see also mdn/stumptown-content#106). The actual items in this case seem simple to support (just booleans). These macros are specific to certain structured page types, so I don't think we should expose them to guide pages as "directives".
Remove These are all "badge/banner" macros. Banners add specially styled blocks of text, while badges add specially styled inline text. See for example: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators#Non-standard_features and https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Array_comprehensions for examples of the "obsolete_*" macros. For "Note", see for example the first paragraph under https://wiki.developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/split#Using_split. I propose removing all three of these. I expect removing
Replace These are used to build spec tables. They are already supported. We don't need to expose them to guide pages as directives.
Replace Already supported. We don't need to expose them to guide pages as directives.
Replace Embeds a test report table in an iframe. See for example https://wiki.developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/globalThis#Implementation_progress. If we want to keep this feature, we'll need to replace this macro. We don't need to expose them to guide pages as directives.
Replace We already support this, and it's enabled for guide pages.
Replace We already support this, and it's enabled for guide pages.
Render This page is used to include content from one page in another page. I would like us to render out this macro, in general. I think ad hoc transcluding of content is a kind of content antipattern (mostly because the original source of the doc doesn't know where it is included, so it's easy to edit it so it doesn't make sense in the transcluding context). But it is used quite extensively in the JS docs to include instance properties and methods from the corresponding For example, https://wiki.developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray uses it to include instance properties from https://wiki.developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray/prototype. I think we should consider whether the JS docs really want to be doing this. If they do, we should find a way to support it.
Remove This is used in pages like
gives output like:
So the macro converts "33" -> "(Firefox 33 / Thunderbird 33 / SeaMonkey 2.30)". Really, that paragraph should be removed, and its contents represented in BCD if it's important. But that's a manual update. For the sake of the machines, it would be fine to just remove the macro call, and the output would be:
Replace, after changing to use EmbedLiveSample Embeds a JSFiddle in the page. These are only used in one page, https://wiki.developer.mozilla.org/en-US/docs/Web/JavaScript/Closures. Using JSFiddle embeds doesn't add any advantages over a normal live sample, as far as I can tell.
Replace These are used to build two landing pages: I'd suggest we rework these pages to be "stumptown landing page compatible", and scrape them as landing pages. We can I hope use stumptown "link lists" to replace the actual macro calls here.
Remove Used in one page: https://wiki.developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/search.
Replace Used in one page: https://wiki.developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors, which looks like it wants to be a landing page. I'd suggest we rework this page to be "stumptown landing page compatible", and scrape it as as a landing page. Listing subpages is already supported in stumptown "link lists".
Replace ?? Used in only one page, https://wiki.developer.mozilla.org/en-US/docs/Web/JavaScript/Index. I think these pages are used for DocStatus, and I assume that in GitHub that whole system will be replaced. However we haven't yet thought about what the requirements are for this or how we will do it. SummarySo to summarise, I'd say: Remove:
Render:
Replace:
[1] already supported in stumptown |
This is great. Thank you for writing all of these up, Will. There was only one macro decision that I had misgivings about and that was replacing I take it we'll have separate issues to hash out the implementation details of removing or rendering out specific macros? For example, I'd like to see the potential diff of removing |
I'd be happy to remove it, but it is very new (mdn/kumascript#1245). As I understand it, it is experimental and I don't know what the status of the experiment is. I think Florian's input is needed here.
Well, some of this work has to be done now, just to make a sensible determination. I mean, we need to have some idea of what they do and where/how they are used, and I've done some of that digging in the comment above. And I do want to be careful in this step and get to quite a high degree of confidence that we are making good decisions. But yes, there are issues for the next steps (e.g. https://github.com/mdn/sprints/issues/2524) and I don't expect us to get all the decisions right at this point. Does that answer your concern? |
Great work here @wbamberg ! I agree with most of it, and just had the odd few comments/queries:
|
Thanks Chris, for taking a look!
Some difficulties with this:
One reason for suggesting we abuse code fencing is that you get something like preview support, so source like this:
is shown in my editor's preview like this: I think in most cases, removing notes from MDN pages improves them. What happens is that people have A Thing they want to say, and it's important to them, because they're motivated enough to add it, so they mark it up to show how important it is. But when everyone does that, it's the documentation equivalent of a party where everyone's talking at once, so some people start talking louder to be heard, then eventually everyone's shouting and you can't hear anything. For example, see the section of the JS docs that use I think this passage would be improved by just removing the Note styling.
I think they are used only by localizers, and @SphinxKnight has told me that they are useful and used. And yes, I expect this would have an out-of-content solution in stumptown, but that doesn't absolve us from thinking about it :). |
Valid points here, thanks for taking the time to explain the difficulties in so much detail. It is taking me a while to get into the stumptown mindset when thinking about how things will work, and this is very helpful! So yes, the code fencing thing does look it could work. I totally agree with you on note usage too; as you said before, the content should largely just speak for itself. |
OK, that's fine. If you're pretty confident that the macros can be removed safely now, then that's good enough for me. Also, I mis-wrote before: my hope was that for macros that get rendered out to have an annotation as such (e.g., a |
This is really exciting progress and looks great to me, thanks @wbamberg, @Elchi3, @ddbeck, and @chrisdavidmills!! I had one thought/question while reading through the macro decisions. I was wondering if it might make more sense to remove the |
That's a very interesting comment @escattone ! In this case I agree with you, remove is better. But as you imply, If we had a tool that let us say: "remove {{page}} only for pages under /en-US/docs/Web/JavaScript", and we could be fairly sure it's only called in this context (which means we have to spend the time checking through all 142 calls to this macro), then I'd be happy to do this for JS. So this also suggests we could triage macros differently in different sections of the docs. |
@wbamberg I did some analysis of all of the documents under
|
There is a lot to unpack for me in here, but I'm not really disagreeing with anything said or discussed. Very nice work, Will! On the
Yes, but I'm planning to get rid of this completely. So, I guess it is "replace" and the implementation of "replace" has been hashed out in London already: mdn/sprints#2571. For the remaining usage of {{page}} (that are not Foo/prototype pages), I think we probably remove or render. Need to see this case by case, but I don't want to support transclusion unnecessarily at this point. On the
We need to get results back from Bocoup and then we need to make a call. I agree with y'all about the questionable usefulness, but I'd like to also give the metrics a chance to speak for themselves first. On Let's remove all this. If needed, we will have to re-invent these tools. |
Thanks for the reviews everyone. I'm going to summarize our choices below and close this issue. Remove:
Render:
Replace:
To do
[0] many calls to this macro will be removed in the course of mdn/sprints#2571 |
We did some work on this in Berlin, and:
So I'd like to reopen this issue with an updated proposal: Retain/replace:Non-standard_Header [0] Render:jsxref [1] RemoveDraft To do:EmbedTest262ReportResultsTable Notes[0] These are all what we sometimes call "banners". They create a banner across the page warning that the item is (for example) deprecated. See e.g. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Expression_closures for a nice pile of these things. We had triaged these as "remove", but Florian pointed out that without them, it would sometimes be hard to notice this important attribute. It should be present in BCD, but the way this is represented in the rendered page makes it very easy to miss. Currently it's a problem that this attribute is recorded in two or more places - the BCD itself and the macro call. Ideally, the attribute would only be recorded in BCD, and the renderer would add the header/banner automatically. So the proposal is to keep these macros in Kuma, and update stumptown so it can automatically set the header based on the BCD. Then the scraper's "replace" algorithm is just to remove the macros. [1] These are xrefs. Before rendering them, we would like to update the macro so it does not include the [2] These are what we sometimes call "badges". They are supposed to convey similar information to banners, but appear next to the link to the page for the item, rather than on the page itself. They are typically rendered as icons like 👎 . I think it would be OK to just remove these - anyone clicking the link will see that the item is deprecated. |
I'm happy with this update. I'm going to spend some more time this week on improving the linter in the hopes that we'll reliably catch macros (I think I've got an approach that's workable). |
Makes sense to me. Thanks Will for capturing this discussion so nicely! |
{{page}} is actually annoying and I think will involve case-by-case work. Very often it seems like it could be replaced with a link. Sometimes (e.g. in https://wiki.developer.mozilla.org/en-US/docs/Web/JavaScript/Reference$edit ) it should be replaced by some landing page magic. Sometimes probably it should be rendered. |
I've filed this as mdn/sprints#2787 now. |
We've discussed this elsewhere, but for the record: We might want to remove this as some future cleanup operation. "optional_inline" is called 198 times in the JS docs, and from 689 pages in all our docs (en-US). Which, while it is a lot, seems fewer than we would expect if it were being used consistently. |
Updated triage list, based on changes mentioned in https://github.com/mdn/sprints/issues/2520#issuecomment-593684240: Retain/replace:Non-standard_Header Render:jsxref RemoveDraft To do:EmbedTest262ReportResultsTable |
Taking this issue out of the Tracy Chapman milestone, so I can close that milestone. |
Regarding the list of "Remove" (above)...
So let's compare that list with what we've got:
(in other words, at the time of writing, all of these macros render out and are not cleaned up with the flaw-system) A tricky one is |
Also, can we not say that this issue is complete and can closed? If Will's comment above is the final verdict, then that answers the issue's title and the next action is to explode this into fresh new smaller atoms of issues that we can work against. |
@schalkneethling asked for input on this. So here are my thoughts. I think we have less macros these days but not all can be removed yet. (I haven't done a grep on macro usage but suspecting the list above is more or less accurate).
|
|
If I may (re: mdn/translated-content)
|
I've been thinking about this a bit recently. It feels like something changed in Yari that made the syntax for xrefs more complicated. I think there used to be a reasonable argument that xref syntax was simpler even than Markdown links, but often these days I find myself writing something like:
and wondering in which ways that is better, syntactically, than:
As people have said upthread: given a consensus that we want to do this, it would be quite a mechanical task. According to the KS landscape doc this is the second biggest group of macros (almost certainly the biggest now, since most of the "internal" group should have been removed or be removable). {{previous}} and {{previousnext}} are different though because they want particular styling, so plain Markdown won't work. Stumptown had an abstraction called a "chapter list" which was intended to enable auto-generation of these types of links: mdn/stumptown-content#265. |
We have deleted/deprecated a lot of macros since then. But this is still valid, even if Dealing with banners would be a contained (and doable) project (on the dev side), to move this project forward, while we continue to remove the use of the long tail of macros. Sidebars macros depend on the page-type project (adding page-type to the YAML header) and the renaming of the page (which will add another YAML header). Once this is done, we should be mature enough to start a sidebar project.
|
I have no idea what is going on. There are a LOT of issues being closed by the |
Most of these macros have already been replaced, and this is obsolete with the rari work, so I'm closing this as resolved. |
Depends on mdn/sprints#2519
These decisions should be made public and reviewed by our content lead.
Acceptance criteria
We have a resolution (render, remove, replace) for every macro called from the JS docs.
The text was updated successfully, but these errors were encountered: