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

Give the readme a little love #163 #258

Merged
merged 2 commits into from
Feb 25, 2017
Merged

Give the readme a little love #163 #258

merged 2 commits into from
Feb 25, 2017

Conversation

ghost
Copy link

@ghost ghost commented Feb 25, 2017

Give the readme a little love #163

Give the readme a little love #163
@ghost ghost requested a review from iteles February 25, 2017 15:14
Copy link
Member

@iteles iteles left a comment

Choose a reason for hiding this comment

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

Thanks for this! Also looking forward to seeing that issue be closed 😉 (we can come back to it)

Would like to see a tiny change in the order that the information is presented. I firmly believe developers read the first sentence and then skim the rest - I don't think our first sentence quite reflects what we want to convey to our github audience 😊

README.md Outdated

# Why?
DWYL LTD is incorporated and registered in England and Wales with company number 09525434 and registered office is at DWYL LTD, 104 Mary Datchelor Close, London, SE5 7DY, United Kingdom.
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 this needs a tiny bit of expansion, I always talk about first and foremost being an open source community and that is the friendliness I'd like to convey, see: dwyl/start-here#117

I think that that and the mission is way more important than the registered company for now, particularly with regards to our github audience (vs our website audience which is more client-focused) and should be the first thing people read 😊

Copy link
Author

Choose a reason for hiding this comment

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

I thought it would be handy considering it's the ops and admin repo

Copy link
Member

Choose a reason for hiding this comment

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

I agree it's important info, particularly the registered company no. and address, I'm just requesting we move the info around to reflect what we want our audience to see first - most people who get here don't know dwyl as anything more than an awesome online resource!

@iteles iteles assigned ghost Feb 25, 2017
@iteles iteles merged commit e2dfd06 into master Feb 25, 2017
Copy link
Member

@iteles iteles left a comment

Choose a reason for hiding this comment

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

I feel like I just wore you down here 😆

Thanks for the change!!

@iteles iteles added the in-review Issue or pull request that is currently being reviewed by the assigned person label Feb 25, 2017
@iteles iteles deleted the readme-love branch February 25, 2017 18:40
@iteles iteles removed the in-review Issue or pull request that is currently being reviewed by the assigned person label Feb 25, 2017
@ghost
Copy link
Author

ghost commented Feb 25, 2017

Is it possible for reviewers to edit the file directly rather than suggest changes? I wouldn't be offended if you wanted to make a change like that when reviewing! I misread your first message and thought you didn't want it rather than just moving it down

@iteles
Copy link
Member

iteles commented Feb 25, 2017

@markwilliamfirth It is, but then it would still require a review from you.

For small changes like this and typos it's probably the better way 👍

@ghost
Copy link
Author

ghost commented Feb 25, 2017

The former might be more beneficial - probably would have been quicker (in terms of our combined time) rather than explaining etc

with me reviewing i'd see and understand what you'd want changing

@jammur jammur unassigned ghost Dec 14, 2017
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.

1 participant