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

Improve the clarity of the readme markdown file. #52

Merged
merged 2 commits into from
May 29, 2024
Merged

Conversation

georgeliao
Copy link
Contributor

No description provided.

Copy link
Collaborator

@ricab ricab left a comment

Choose a reason for hiding this comment

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

Hey @georgeliao, thanks for improvement. I am fine with the clarificatoin you added in the text, but I am not convinced by the update to the link.

README.md Outdated
@@ -56,10 +56,10 @@ you would need to override the systemd service with the following setting:

```conf
[Service]
Environment="MULTIPASS_BLUEPRINTS_URL=https://github.com/USERNAME/multipass-blueprints/archive/refs/heads/BRANCH_NAME.zip"
Environment="MULTIPASS_BLUEPRINTS_URL=https://github.com/canonical/multipass-blueprints/archive/refs/heads/<BRANCH_NAME>.zip"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This wouldn't be valid for a fork (which a contributor without write access to the repository needs to use).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, I missed the context a bit.
USERNAME can mislead to the actual github user name of ours for the folk in Canonical. Meanwhile, USERNAME/organization_name seems to be the right name for that part of the URL. So I also do not have a good way to disambiguate this. Maybe I can change it back and just add<> to emphasize it is a variable?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Right. How about <REPO_OWNER>?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds better.

Copy link
Collaborator

@ricab ricab left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@ricab ricab merged commit 66b723c into main May 29, 2024
2 of 4 checks passed
@ricab ricab deleted the improve_readme branch May 29, 2024 13:52
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