-
Notifications
You must be signed in to change notification settings - Fork 38
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
Add two new config options for review handling #47
Conversation
* `default_owner_slack` means you can set a single owner which applies to all pages without a specific `owner_slack` item in the frontmatter. * `owner_slack_workspace` allows customisation of the workspace to link to when an owner is present.
2d8f2dc
to
4fe8a4e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Docs requests, rather than anything wrong.
docs/configuration.md
Outdated
default_owner_slack: '#owner' | ||
``` | ||
|
||
## `owner_slack_workspace` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For clarity, can we have the docs the other way around - workspace
followed by the channel entry?
docs/configuration.md
Outdated
@@ -156,3 +156,21 @@ Define a path to an Open API V3 spec file. This can be a relative file path or a | |||
```yaml | |||
api_path: ./source/pets.yml | |||
``` | |||
|
|||
## `default_owner_slack` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you note that this (or the page override) is required if owner_slack_workspace
is set? Also, can you add both of these config options to the example tech-docs.yml
file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I'm being picky, I'd say this variable name sounds like a reference to a workspace, not a channel / user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name is based on the existing frontmatter item called owner_slack
. I don't really want to change that one too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you note that this (or the page override) is required if owner_slack_workspace is set?
It's not: if neither is set then the owner just won't be linked.
3ab710b
to
a8824d8
Compare
default_owner_slack
means you can set a single owner which applies to all pages without a specificowner_slack
item in the frontmatter.owner_slack_workspace
allows customisation of the workspace to link to when an owner is present.