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

Fix comments in config file #1

Merged
merged 2 commits into from
Feb 13, 2016
Merged

Fix comments in config file #1

merged 2 commits into from
Feb 13, 2016

Conversation

FryDay
Copy link
Contributor

@FryDay FryDay commented Feb 9, 2016

Some of the defaults in the comments do not match the actual default values.

@MaxLeiter
Copy link
Member

Looks good to me

@AlMcKinlay
Copy link
Member

Yep, lgtm too.

@@ -30,7 +30,7 @@ module.exports = {
// Set the local IP to bind to.
//
// @type string
// @default "0.0.0.0"
// @default undefined
Copy link
Member

Choose a reason for hiding this comment

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

Could this perhaps explain what undefined means in this context?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, that's probably a good plan. Just a short explanation that undefined means it'll listen on everything.

@AlMcKinlay AlMcKinlay self-assigned this Feb 12, 2016
@astorije
Copy link
Member

Hey @FryDay, could you get rid of the merge commit in the history please? Let me know if you don't know how to do that and we can sync on IRC.

In the future, when you want to sync with master, always use git rebase master instead of merge or pull (pull is simply fetch + merge).

Thanks a lot for fixing cruft!!! 😄

@astorije astorije added Type: Bug Issues that report and PRs that solve any defects that cause unexpected behaviors. Type: Documentation Lack of documentation, improvement suggestion, or PRs that address these. labels Feb 13, 2016
@FryDay
Copy link
Contributor Author

FryDay commented Feb 13, 2016

@astorije certainly, I'll take care of it tonight when I get home.

@astorije
Copy link
Member

Perfect :-)

@FryDay
Copy link
Contributor Author

FryDay commented Feb 13, 2016

@astorije I believe I fixed it. If it doesn't look okay, let me know.

@astorije
Copy link
Member

I'm 👍 on this, going for second review (as there were changes after @YaManicKill's comment).
@FryDay, maybe squash the commits into one if you happen to stop by before @YaManicKill gives his 👍, otherwise not a huge deal.

@AlMcKinlay
Copy link
Member

👍

I'm fine with teh 2 commits because they are both doing logically seperate things. Merging.

AlMcKinlay added a commit that referenced this pull request Feb 13, 2016
Fix comments in config file
@AlMcKinlay AlMcKinlay merged commit 3d159f6 into thelounge:master Feb 13, 2016
@astorije astorije assigned AlMcKinlay and unassigned AlMcKinlay Feb 14, 2016
@astorije astorije added this to the 1.0.1 milestone Apr 1, 2017
@AlMcKinlay AlMcKinlay removed their assignment Mar 12, 2018
@ghost ghost mentioned this pull request Jun 17, 2019
@lcarva lcarva mentioned this pull request Aug 20, 2020
brunnre8 pushed a commit to brunnre8/thelounge that referenced this pull request Apr 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Issues that report and PRs that solve any defects that cause unexpected behaviors. Type: Documentation Lack of documentation, improvement suggestion, or PRs that address these.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants