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

Update instructions for adding new macros #10513

Merged
merged 7 commits into from
Dec 23, 2021
Merged

Update instructions for adding new macros #10513

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6f6ecaa
Update instructions for adding new macros
astearns Nov 14, 2021
00f18e3
lint fixes
astearns Nov 14, 2021
e5b6402
macro/value distinction
astearns Nov 14, 2021
c419c7c
values, not strings
astearns Nov 14, 2021
817bfae
Perhaps do it this way?
hamishwillee Nov 15, 2021
4aac803
Update to: Making a change that depends on external content
hamishwillee Nov 29, 2021
353b80a
Fix md-lint errors, hopefully
wbamberg Dec 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 21 additions & 21 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -707,27 +707,27 @@ with the [content mozilla directory](https://github.com/mdn/content/tree/main/fi
Both locations represent the same directory structure, but the
`index.html` file appears in the latter, therefore the page is not archived.

### Making a change that depends on a macro update

KumaScript macros are still used on MDN pages, even in the new platform.
These are the function names surrounded by handlebars-style double curly
braces that you'll see in the source code on occasion, for example
`{{domxref}}` Eventually we have to replace them with something else,
but they are here for now. They live in <https://github.com/mdn/yari/tree/main/kumascript/macros>.
Copy link
Collaborator

@hamishwillee hamishwillee Nov 14, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

This change is incorrect. The jsondata did indeed move, but the macros did not, and this sentence is referring to the macros.

I like where you're heading with this though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see what you mean. Does the latest change make more sense?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@astearns Yes it makes more sense and this is an improvement.

But even so, I have modified this even more ruthlessly. The only macro for which this is particularly relevant is the API sidebar one (i'm not sure which of the other data is actually used since specdata now comes from browser compatibility table).

So I have updated this to point to highlight that, and point to more detailed instructions on how it that is updated. I removed the particular example PR because to me that is just an example of change, not instruction on how to change.

This section probably needs a link for what each type of data is used for an what macros it effects, but IMO this is good for now.

Thanks very much for highlighting this.

@teoli2003 Deferring to you to further review

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FWIW, my original problem was needing to figure out how to use the macro to link to a new spec section. I likely would not have even noticed this part of the ReadMe without the SpecRef.json example.

The spec link and compatibility table are incorrect on this page: https://developer.mozilla.org/en-US/docs/Web/API/CSS

I had been assuming I’d need to create a new SpecRef.json entry for that page, but now I am not sure if that is correct.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@astearns Those tables are filled from data in browser compatibility which is specified here in the metadata : https://github.com/mdn/content/blob/main/files/en-us/web/api/css/index.md?plain=1#L10 - see browser-compat: api.CSS

That data is actually defined here: https://github.com/mdn/browser-compat-data/blob/main/api/CSS.json

I am not sure how SpecData is used, which is why I've asked for review of my comment :-).

Copy link
Collaborator

@hamishwillee hamishwillee Nov 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

PS But it does highlight the need to make it clear where browser compat data comes from, if it is not documented in this page. Happy to explore that when we know what the other json files are used for in the tree.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@wbamberg I was wondering if you could review this? It's been open for a bit.

I think it is correct. My concern is whether more needs to be done, and that depends a bit on how the json in https://github.com/mdn/content/blob/main/files/jsondata/ is used. I THINK perhaps specdata is no longer used, and perhaps some of the others.
If you don't know, who do you think might?

Copy link
Collaborator

@wbamberg wbamberg Nov 26, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Sorry, this is going to be a long answer to an apparently simple question.

On the specifics of SpecData: yes, it is still used 😭. It's used by the Spec2 and SpecName macros, which were the old way to build spec tables, before we started keeping spec URLs in BCD. Looking through our content with something like rg Spec2 ./files you'll still see a lot of uses of these macros.

Without more detailed work I don't know how many of these can be removed, and why they can't reference a spec URL in BCD. I do know that some pages can't reference spec URLs in BCD because BCD doesn't have an entry that maps to them - API overview pages, for example. This is related to https://github.com/mdn/content/discussions/4920 and I think my preferred approach there would be to allow spec URLs in the page front matter as a fallback for API pages (and maybe also for other pages whose features aren't in BCD). But like I say, more work is needed there. It would be nice to resolve this.

Most of the time though, spec URL updates should happen in BCD and pages should be able to pull them from there.

On this PR: I do agree that the example given here:

Take #187. Florian wanted to add documentation for a new WebGL extension to MDN...

...is out of date and misleading and needs to be fixed. But this PR changes the section from "Making a change that depends on a macro update" to "Making a change that depends on a JSON data update", and these are not the same thing at all. Many macro updates are not updates to files under "files/jsondata". For example, adding a new page in the Learning Area requires a corresponding update to the sidebar, which is a macro under yari/kumascript.

Also though, this README section is short and vague enough as to be not much help for contributors who don't already understand the byzantine complexity of MDN's authoring system. So they're going to have to ask for help anyway.

So I think there are basically two options for this issue:

  • just replace that example with one about updating a sidebar. Then at least it's not actively misleading.

  • update that section to be something like "Making a change that depends on external content", and say something like: sometimes MDN content updates require updates to other files and repositories, such as KS macros, or JSON data, or mdn/data, or external example repos like mdn/learning-area. In these situations make the PR to the other repo first and get that merged. I guess an exception actually is JSON data now, because it's now in the content repo too so can be updated on the same PR.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks @wbamberg - sounds like good advice. I've gone with option 2 - is this now better?


Sometimes you'll want to make a change to the content that relies on a
macro change. Take [https://github.com/mdn/content/pull/187](https://github.com/mdn/content/pull/187).
Florian wanted to add documentation for a new WebGL extension to MDN, but
this change relied on the new feature's spec being added to
<https://github.com/mdn/yari/blob/main/kumascript/macros/SpecData.json>.
If not, the specification table on the new page would not render properly
because the data it relies on would not be there.

In such situations:

1. Make the required PR to <https://github.com/mdn/yari/tree/main/kumascript/macros>
first, and get that merged.
2. Add the content to this repo.
### Making a change that depends on external content

Some MDN content is created from external data files or repositories using KS macros.
Generally you should create a PR to first update the external content before
updating the associated MDN pages.

Relevant external content includes (non-exhaustively):

- [GroupData.json](https://github.com/mdn/content/blob/main/files/jsondata/GroupData.json):
Data definition of
[Sidebars](https://developer.mozilla.org/en-US/docs/MDN/Contribute/Howto/Write_an_API_reference/Sidebars)
used by `{{APIRef}}` macro.
- [https://github.com/mdn/interactive-examples](https://github.com/mdn/interactive-examples):
Source code repo for interactive examples inserted using the
`{{EmbedInteractiveExample}}` macro.
- [https://github.com/mdn/learning-area](https://github.com/mdn/learning-area):
Source code repo for examples referenced in the
[Learning Area](https://developer.mozilla.org/en-US/docs/Learn)
- [JSON structured data](https://developer.mozilla.org/en-US/docs/MDN/Guidelines/JSON_Structured_Data)
provides more information about JSON data used to define inheritance diagrams
(`{{InheritanceDiagram}}` macro), specification tables, etc.

## Frequently asked questions (FAQ)

Expand Down