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

DOC Update syntax for callout blocks #452

Conversation

GuySartorelli
Copy link
Member

IMPORTANT

This should not be merged until all of the associated PRs have all been approved. Only merge when all of the PRs are ready to go, to minimise the amount of time spent with broken callout blocks.

Description

Converts all of the legacy callout blocks to the new syntax. Note that we're not doing CMS 3.

There is a separate PR for CMS 4. Lots of the changes will be the same, but there's been enough change between majors that a direct merge up will be a pain in the butt, so I've done two separate PRs. Once they're both merged the merge-up process will literally be to throw away the differences and keep the CMS 5 copy.

Note that most of the callout blocks will be converted as per silverstripe/doc.silverstripe.org#282 (comment) - but there are some which were intentionally changed. I will note those with comments.
If I haven't added a comment on something that has been converted differently than that table suggests, assume I messed up.

Issues

Pull request checklist

  • The target branch is correct
  • All commits are relevant to the purpose of the PR (e.g. no TODO comments, unrelated rewording/restructuring, or arbitrary changes)
    • Small amounts of additional changes are usually okay, but if it makes it hard to concentrate on the relevant changes, ask for the unrelated changes to be reverted, and submitted as a separate PR.
  • The commit messages follow our commit message guidelines
  • The PR follows our contribution guidelines
  • The changes follow our writing style guide
  • Code examples follow our coding conventions
  • CI is green

@GuySartorelli GuySartorelli marked this pull request as draft January 30, 2024 23:05
@GuySartorelli GuySartorelli force-pushed the pulls/5/update-callout-syntax branch 2 times, most recently from ce20619 to 5723016 Compare January 30, 2024 23:55
Comment on lines +290 to +291
> [!NOTE]
> `exclude` directives will always supersede `include` directives.
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved this from below to satisfy MD028 - can't have two block quotes immediately after one another.

Comment on lines +67 to +70
> [!NOTE]
> Note the use of the default arguments on `date`. Fields created from `DBFields`
> generate their own default sets of arguments. For more information, see
> [DBFieldArgs](query_plugins#dbfieldargs).
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved this from below to satisfy MD028 - can't have two block quotes immediately after one another.

Comment on lines +615 to +616
> [!WARNING]
> It is critical that you end series of mutation calls with `getState()`.
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved this from below to satisfy MD028 - can't have two block quotes immediately after one another.

Comment on lines +257 to +258
> [!WARNING]
> Because of the unique structure of the `form` middleware, you cannot register new services to `Injector.form`.
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved this from below to satisfy MD028 - can't have two block quotes immediately after one another.

Comment on lines +378 to +379
> [!NOTE]
> Note that trying to call `$('div').bar();` would throw an uncaught `TypeError` saying something like "$(...).bar is not a function", because the `bar()` function was defined in a namespace, but we are trying to call that function from *outside* of that namespace.
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved this from below to satisfy MD028 - can't have two block quotes immediately after one another.

Comment on lines +592 to +593
> [!TIP]
> Note that in the below examples we just return the `DataList` without executing it. We don't need to execute the query, the reading mode is attached to the `DataList` as soon as it's created via the [`augmentDataQueryCreation()`](api:SilverStripe\Versioned\Versioned::augmentDataQueryCreation()) extension hook implementation.
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved this from below to satisfy MD028 - can't have two block quotes immediately after one another.

Comment on lines +124 to +126
> [!WARNING]
> Versioning only works if you are adding the extension to the base class. That is, the first subclass
> of `DataObject`. Adding this extension to children of the base class will have unpredictable behaviour.
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved this from below to satisfy MD028 - can't have two block quotes immediately after one another.

Comment on lines +65 to +66
> [!WARNING]
> Because of the way the ORM interacts with class inheritance, some models will spread their data across multiple tables. See [Joining tables for a DataObject inheritance chain](#joins-for-inheritance) below for information about how to handle that scenario.
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved this from below to satisfy MD028 - can't have two block quotes immediately after one another.

Comment on lines +152 to +153
> [!TIP]
> All of the below methods to get a single record will return `null` if there is no record to return.
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved this from below to satisfy MD028 - can't have two block quotes immediately after one another.

[hint]

- returning `$this` is the equivalent of returning an empty array.
- returning `$this->customise()` is the equivalent of returning an array with data.

[/hint]
> [!TIP]
> There are a couple of things to note here:
>
> - returning `$this` is the equivalent of returning an empty array.
> - returning `$this->customise()` is the equivalent of returning an array with data.
Copy link
Member Author

Choose a reason for hiding this comment

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

Added text above the list because we need a blank link before the list to satisfy MD032, and it would look weird with just a blank spot there.

Copy link
Member

@emteknetnz emteknetnz left a comment

Choose a reason for hiding this comment

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

Shouldn't we be targeting 5.1 instead of 5?

@GuySartorelli GuySartorelli changed the base branch from 5 to 5.1 February 1, 2024 01:23
@GuySartorelli GuySartorelli force-pushed the pulls/5/update-callout-syntax branch from 5723016 to 83719af Compare February 1, 2024 01:24
@GuySartorelli
Copy link
Member Author

Yes, oops. Cherry picked on top of 5.1 and resolved the few conflicts.
After this is merged and merged up I'll need to do one last PR on 5 for the new pages or any other new content on 5 that isn't on 5.1

@GuySartorelli GuySartorelli force-pushed the pulls/5/update-callout-syntax branch from 83719af to 5917947 Compare February 1, 2024 01:29
@emteknetnz emteknetnz merged commit e77dd47 into silverstripe:5.1 Feb 1, 2024
3 checks passed
@emteknetnz emteknetnz deleted the pulls/5/update-callout-syntax branch February 1, 2024 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants