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

Development setup in readme, and some adjustments #12192

Merged
merged 9 commits into from
Nov 1, 2018

Conversation

jancborchardt
Copy link
Member

As mentioned in #12186 (comment), adding some info regarding frontend development setup to the main readme.

Also fix some structure of readme, adjust wording, and add more emojis 🤩

Please review @skjnldsv @juliushaertl @camilasan @rullzer @danxuliu @MariusBluem

Signed-off-by: Jan-Christoph Borchardt <hey@jancborchardt.net>
Signed-off-by: Jan-Christoph Borchardt <hey@jancborchardt.net>
Signed-off-by: Jan-Christoph Borchardt <hey@jancborchardt.net>
README.md Outdated

1. [Set up your local development environment](https://docs.nextcloud.com/server/14/developer_manual/general/devenv.html) :rocket:
2. [Pick a good first issue](https://github.com/nextcloud/server/labels/good%20first%20issue) :notebook:
3. Create a branch, a [Pull Request](https://opensource.guide/how-to-contribute/#opening-a-pull-request) and `@mention` the people from the issue :computer:
4. Wait for it to get merged and :tada:
3. Create a branch, a [pull request](https://opensource.guide/how-to-contribute/#opening-a-pull-request) and `@mention` the people from the issue to review :computer:
Copy link
Member

Choose a reason for hiding this comment

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

Can we add a note about adding the sign off message here? This is not obvious from the readme and missing in a lot of first time contributors PRs.

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! Is it good like that?

Signed-off-by: Jan-Christoph Borchardt <hey@jancborchardt.net>
Signed-off-by: Jan-Christoph Borchardt <hey@jancborchardt.net>
@jancborchardt
Copy link
Member Author

Also added sentence about non-technical contributions with link to https://nextcloud.com/contribute/, and fixed the screenshot! (@jospoortvliet @MariusBluem if the structure in the Screenshots repo is changed, links will break. ;) We should keep the naming.)

Signed-off-by: Jan-Christoph Borchardt <hey@jancborchardt.net>
Copy link
Member

@rullzer rullzer left a comment

Choose a reason for hiding this comment

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

Nice :)

README.md Outdated

[![BrowserStack](https://user-images.githubusercontent.com/45821/41675934-61fa3442-74c4-11e8-8c8e-90768c56ba08.png)](https://www.browserstack.com/)
[![WAVE](https://wave.webaim.org/img/wavelogo.png)](https://wave.webaim.org/extension/)
[![](https://developers.google.com/web/tools/lighthouse/images/lighthouse-icon-128.png) Lighthouse](https://developers.google.com/web/tools/lighthouse/)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
[![](https://developers.google.com/web/tools/lighthouse/images/lighthouse-icon-128.png) Lighthouse](https://developers.google.com/web/tools/lighthouse/)
[![Lighthouse](https://developers.google.com/web/tools/lighthouse/images/lighthouse-icon-128.png)](https://developers.google.com/web/tools/lighthouse/)

Copy link
Contributor

Choose a reason for hiding this comment

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

Personally i think there are to many smileys now 😁. Anyway i think the logos under "Tools we use" should be a bit smaller or even same size.

Copy link
Member Author

Choose a reason for hiding this comment

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

@splitt3r I see why you suggest that. :) This is intentional though as the logo I found has no text. And the others do have text in the logo. So for Lighthouse it is indeed text, and not just alternative text for the image.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, makes sense.

Copy link
Member Author

Choose a reason for hiding this comment

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

Anyway i think the logos under "Tools we use" should be a bit smaller or even same size.

Good point! Switched to using HTML in that section and reduced the size.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually didn’t really work out with the HTML – just switched to text-only. :)

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if browserstack required to include the logo for the free accounts, but @rullzer probably knows.

Signed-off-by: Jan-Christoph Borchardt <hey@jancborchardt.net>
@jancborchardt jancborchardt dismissed splitt3r’s stale review November 1, 2018 15:18

See comment, it’s intentional :)

Signed-off-by: Jan-Christoph Borchardt <hey@jancborchardt.net>
Signed-off-by: Jan-Christoph Borchardt <hey@jancborchardt.net>
@jancborchardt
Copy link
Member Author

Test failures are irrelevant cause this is only a readme file.

@jancborchardt jancborchardt merged commit 6702576 into master Nov 1, 2018
@jancborchardt jancborchardt deleted the development-setup branch November 1, 2018 23:23
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 design Design, UI, UX, etc. enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants