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 readme #35691

Closed
wants to merge 6 commits into from
Closed

Improve the readme #35691

wants to merge 6 commits into from

Conversation

clivinghouse
Copy link

@clivinghouse clivinghouse commented Dec 9, 2022

Signed-off-by: chl5158 79345446+clivinghouse@users.noreply.github.com

Summary

This is my first pull req. I have skimmed over the requirements to contribute to this project, I believe I have met the projects requirements. Please let me know if I have missed anything! I have only made some grammatical edits. Couple added periods here, some sentence restructuring there.

TODO

  • Review of grammar.
  • make sure edits don't misconstrue previous meanings.

Checklist

- Code is properly formatted N/A

  • Sign-off message is added to all commits
    - [X ] Tests (unit, integration, api and/or acceptance) are included N/A
    - [ ] Screenshots before/after for front-end changes No Changes
    - [ ] Documentation (manuals or wiki) has been updated or is not required Directly changing docs N/A
    - [ ] Backports requested where applicable (ex: critical bugfixes) N/A

Signed-off-by: chl5158 <79345446+clivinghouse@users.noreply.github.com>
@szaimen szaimen added this to the Nextcloud 26 milestone Dec 9, 2022
@szaimen szaimen added the 3. to review Waiting for reviews label Dec 9, 2022
@szaimen szaimen requested a review from a team December 9, 2022 06:56
@szaimen szaimen changed the title Edits made Improve the readme Dec 9, 2022
@clivinghouse
Copy link
Author

@szaimen thank you for the title improvement!

README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
- 🖥 [**Install** a server by yourself](https://nextcloud.com/install/#instructions-server) on your hardware or by using one of our ready to use **appliances**
- 📦 Buy one of the [awesome **devices** coming with a preinstalled Nextcloud](https://nextcloud.com/devices/)
- 🏢 Find a [service **provider**](https://nextcloud.com/providers/) who hosts Nextcloud for you or your company
- ☑️ [**Simply sign up**](https://nextcloud.com/signup/) with one of our providers either through our website or through our apps directly.
Copy link
Contributor

Choose a reason for hiding this comment

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

Not our apps, the one of the providers I think

Copy link
Author

Choose a reason for hiding this comment

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

When the sentence talks about signing up, it gives the option of going to the Nextcloud signup page or through "... the apps directly..." i am assuming it means our IOS/Android app. is this sentence to mean

Suggested change
- ☑️ [**Simply sign up**](https://nextcloud.com/signup/) with one of our providers either through our website or through our apps directly.
- ☑️ [**Simply sign up**](https://nextcloud.com/signup/) with one of our providers either through our website or through the providers apps directly.

if it is to mean our IOS/Android App, then maybe it should be more consice

Suggested change
- ☑️ [**Simply sign up**](https://nextcloud.com/signup/) with one of our providers either through our website or through our apps directly.
- ☑️ [**Simply sign up**](https://nextcloud.com/signup/) with one of our providers either through our website or through our IOS and Android apps directly.

Copy link
Member

Choose a reason for hiding this comment

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

No, the change is correct, it's OUR apps (android, iOS, desktop) through which you can sign up with a third party provider ;-) So this is actually good.

Copy link
Author

Choose a reason for hiding this comment

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

Suggested change
- ☑️ [**Simply sign up**](https://nextcloud.com/signup/) with one of our providers either through our website or through our apps directly.
☑️ [**Simply sign up**](https://nextcloud.com/signup/) with one of our providers either through our website or through apps (Android, iOS, Desktop) directly.

would this be good then? @jospoortvliet

README.md Show resolved Hide resolved
6. 🎉 Wait for it to get merged!

Third-party components are handled as git submodules which have to be initialized first. So aside from the regular git checkout invoking `git submodule update --init` or a similar command is needed, for details see Git documentation.
Third-party components are handled as git submodules which have to be initialized first. Git checkout invoking `git submodule update --init` or a similar command is needed, for details see [Git documentation](https://git-scm.com/docs/user-manual).
Copy link
Contributor

Choose a reason for hiding this comment

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

The sentence lost its meaning, it was only missing a comma after «invoking»

Copy link
Author

Choose a reason for hiding this comment

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

I can see your point.

Suggested change
Third-party components are handled as git submodules which have to be initialized first. Git checkout invoking `git submodule update --init` or a similar command is needed, for details see [Git documentation](https://git-scm.com/docs/user-manual).
Third-party components are handled as git submodules which have to be initialized first. So aside from the regular git checkout invoking, `git submodule update --init` or a similar command is needed. For details see [Git documentation](https://git-scm.com/docs/user-manual).

Do you agree to adding a link to the git documentation?

Copy link
Member

Choose a reason for hiding this comment

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

I think that's fine.

Copy link
Member

Choose a reason for hiding this comment

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

But in what you propose, you put the comma wrong. People need to invoke the command 'git submodule' etc, so the coma should be after 'checkout', not after invoking:

So aside from the regular git checkout, invoking git submodule update --init or a similar command is needed. For details see Git documentation

Copy link
Author

Choose a reason for hiding this comment

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

Suggested change
Third-party components are handled as git submodules which have to be initialized first. Git checkout invoking `git submodule update --init` or a similar command is needed, for details see [Git documentation](https://git-scm.com/docs/user-manual).
Third-party components are handled as git submodules which have to be initialized first. So aside from the regular git checkout, invoking `git submodule update --init` or a similar command is needed. For details see [Git documentation](https://git-scm.com/docs/user-manual).

so like this @jospoortvliet?

@clivinghouse
Copy link
Author

@come-nc, thank you for taking the time to look over my pull request. I am going to take a look at your corrections in a bit. I have to complete my finals today. Again, thank you for looking over this!

Copy link
Author

@clivinghouse clivinghouse left a comment

Choose a reason for hiding this comment

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

Waiting for feedback from @come-nc.

- 🖥 [**Install** a server by yourself](https://nextcloud.com/install/#instructions-server) on your hardware or by using one of our ready to use **appliances**
- 📦 Buy one of the [awesome **devices** coming with a preinstalled Nextcloud](https://nextcloud.com/devices/)
- 🏢 Find a [service **provider**](https://nextcloud.com/providers/) who hosts Nextcloud for you or your company
- ☑️ [**Simply sign up**](https://nextcloud.com/signup/) with one of our providers either through our website or through our apps directly.
Copy link
Author

Choose a reason for hiding this comment

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

When the sentence talks about signing up, it gives the option of going to the Nextcloud signup page or through "... the apps directly..." i am assuming it means our IOS/Android app. is this sentence to mean

Suggested change
- ☑️ [**Simply sign up**](https://nextcloud.com/signup/) with one of our providers either through our website or through our apps directly.
- ☑️ [**Simply sign up**](https://nextcloud.com/signup/) with one of our providers either through our website or through the providers apps directly.

if it is to mean our IOS/Android App, then maybe it should be more consice

Suggested change
- ☑️ [**Simply sign up**](https://nextcloud.com/signup/) with one of our providers either through our website or through our apps directly.
- ☑️ [**Simply sign up**](https://nextcloud.com/signup/) with one of our providers either through our website or through our IOS and Android apps directly.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
6. 🎉 Wait for it to get merged!

Third-party components are handled as git submodules which have to be initialized first. So aside from the regular git checkout invoking `git submodule update --init` or a similar command is needed, for details see Git documentation.
Third-party components are handled as git submodules which have to be initialized first. Git checkout invoking `git submodule update --init` or a similar command is needed, for details see [Git documentation](https://git-scm.com/docs/user-manual).
Copy link
Author

Choose a reason for hiding this comment

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

I can see your point.

Suggested change
Third-party components are handled as git submodules which have to be initialized first. Git checkout invoking `git submodule update --init` or a similar command is needed, for details see [Git documentation](https://git-scm.com/docs/user-manual).
Third-party components are handled as git submodules which have to be initialized first. So aside from the regular git checkout invoking, `git submodule update --init` or a similar command is needed. For details see [Git documentation](https://git-scm.com/docs/user-manual).

Do you agree to adding a link to the git documentation?

@blizzz blizzz mentioned this pull request Feb 1, 2023
@clivinghouse
Copy link
Author

since this was mentioned in the next release do i need to have this reviewed again @come-nc?

@come-nc
Copy link
Contributor

come-nc commented Feb 2, 2023

since this was mentioned in the next release do i need to have this reviewed again @come-nc?

No, readme is not really tied to releases to my knowledge.

LGTM :D

Co-authored-by: clivinghouse <79345446+clivinghouse@users.noreply.github.com>
Signed-off-by: Jos Poortvliet <jospoortvliet@gmail.com>
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
clivinghouse and others added 2 commits February 2, 2023 13:38
This is nice. Thank you!

Co-authored-by: Jos Poortvliet <jospoortvliet@gmail.com>
Signed-off-by: clivinghouse <79345446+clivinghouse@users.noreply.github.com>
I was not even aware of this. Thank you for the correction!

Co-authored-by: Jos Poortvliet <jospoortvliet@gmail.com>
Signed-off-by: clivinghouse <79345446+clivinghouse@users.noreply.github.com>
Copy link
Member

@jospoortvliet jospoortvliet left a comment

Choose a reason for hiding this comment

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

See my feedback - once merged/improved, I'm good with this and it can be merged!

Signed-off-by: clivinghouse <79345446+clivinghouse@users.noreply.github.com>
Signed-off-by: clivinghouse <79345446+clivinghouse@users.noreply.github.com>
@skjnldsv skjnldsv mentioned this pull request Feb 23, 2023
@blizzz blizzz mentioned this pull request Mar 7, 2023
@blizzz blizzz modified the milestones: Nextcloud 26, Nextcloud 27 Mar 9, 2023
This was referenced May 3, 2023
@blizzz blizzz mentioned this pull request May 17, 2023
@blizzz blizzz modified the milestones: Nextcloud 27, Nextcloud 28 May 23, 2023
@skjnldsv skjnldsv mentioned this pull request Nov 1, 2023
@blizzz blizzz mentioned this pull request Nov 6, 2023
@clivinghouse clivinghouse deleted the README-edits branch November 10, 2023 04:24
@jospoortvliet
Copy link
Member

thanks @clivinghouse !!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants