Skip to content

Conversation

reitzig
Copy link
Contributor

@reitzig reitzig commented Nov 7, 2017

Fixes #923

Modify it to fit your needs and make sure provided user exists on the system.

Also, make sure that the Git binary is indeed located at `/usr/bin/git` and change the path if necessary.

Copy link
Member

Choose a reason for hiding this comment

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

Let's make this part of the paragraph just before it, I think it'll flow better that way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn't sure about the formatting rules here. Since the two user-related sentences are split across separate paragraphs, it felt odd to attach something very different to one of the two.

Copy link
Member

Choose a reason for hiding this comment

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

🤔

You might have noticed that Git daemon is started here with git as both group and user.

Modify it to fit your needs and make sure provided user exists on the system.

Also, make sure that the Git binary is indeed located at /usr/bin/git and change the path if necessary.

You know what? Let's make all of these the same paragraph. Oh, and can you add the word "the" between "make sure" and "provided user"?

As per @ben's suggestion, the three paragraphs dealing with adapting the systemd config to the system at hand have been compounded into one.
I also got rid of a double "make sure" for style.
@reitzig
Copy link
Contributor Author

reitzig commented Nov 12, 2017

@ben Done. (Not sure if the commit itself triggers a notification; does it?)

@ben
Copy link
Member

ben commented Nov 13, 2017

@ben ben merged commit 6e09183 into progit:master Nov 13, 2017
@ben
Copy link
Member

ben commented Nov 13, 2017

It does, but it's always nice to get a message telling me that you're ready for another review. Thanks, and congrats on your first PR!

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