-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Rephrased the instruction to configure app in readme.md and contributing.md #21673
Conversation
docs/config.md
Outdated
@@ -1,7 +1,6 @@ | |||
# Configuration | |||
|
|||
You can configure the app by copying `config.sample.json` to `config.json` and customising it. The possible options are | |||
described here. If you run into issues, please visit [#element-web:matrix.org](https://matrix.to/#/#element-web:matrix.org) | |||
You can configure the app by making a copy of the `config.sample.json` and renaming it as `config.json` within the same directory. You can also customize it. The possible options are described here. If you run into issues, please visit [#element-web:matrix.org](https://matrix.to/#/#element-web:matrix.org) |
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 this documentation, config.sample.json
is not necessarily at the same level or place as config.json
. The existing language to say copying the sample to the config.json
file felt clear enough?
"You can also customize it." is also somewhat irrelevant when looking at this doc - I think that sentence can be removed.
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.
I meant the config.json
file to be in the same folder as the config.sample.json
(element-web folder).
The existing language implied there was already a config,json file. I had issues interpreting it as well as a few other people from the #element-web:matrix.org group chat. I thought it would be better to clarify it for new contributors.
`config.json` within the same directory. You can also customize it. See the | ||
[configuration docs](docs/config.md) for details. |
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 indentation here appears to have been lost
1. Configure the app by making a copy of the `config.sample.json` and renaming it as | ||
`config.json` within the same directory. You can also customize it. See the | ||
[configuration docs](docs/config.md) for details. | ||
3. `yarn dist` to build a tarball to deploy. Untaring this file will give |
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.
to fit the surrounding document, this would also be numbered at 1
Thank you for the reviews, I'll work on them. |
Closing since there hasn't been any follow-up since May 2022. |
I rephrased the instructions on how to configure the app in the steps to setting up a development environment for easier understanding by new contributors.
Fixes vector-im/element-web/issues/21674
This PR currently has no changelog labels, so will not be included in changelogs.
A reviewer can add one of:
T-Deprecation
,T-Enhancement
,T-Defect
,T-Task
to indicate what type of change this is, or addType: [enhancement/defect/task]
to the description and I'll add them for you.