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.

Sign up for GitHub

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

Jump to bottom

Update node README.md #964

Merged
merged 3 commits into from
Jul 31, 2017
Merged

Update node README.md #964

merged 3 commits into from
Jul 31, 2017

Conversation

styfle
Copy link
Contributor

@styfle styfle commented Jul 19, 2017

@styfle styfle mentioned this pull request Jul 19, 2017
Closed
SimenB
SimenB reviewed Jul 20, 2017
View reviewed changes
node/README.md Outdated
node_modules
```

We have assembled a [Best Practices Guide](./docs/BestPractices.md) for those using these images on a daily basis.
Copy link

Choose a reason for hiding this comment

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

Will a relative link resolve correctly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! Probably not. I fixed this.

@tianon
Copy link
Member

tianon commented Jul 20, 2017

Please take a look at the CI failure -- this needs to be updated in content.md instead. (Almost all the README.md files here are generated, as the comment at the top of each file notes.)

@tianon
Copy link
Member

tianon commented Jul 20, 2017

Also, we use a fork of markdownfmt (see ./markdownfmt.sh in the root of this repo, and Travis's output) -- please follow the formatting it requires/suggests.

@SimenB
Copy link

SimenB commented Jul 21, 2017

If there are changes to the output after running markdownfmt, please PR those backe to node-docker as well 🙂

@styfle
Copy link
Contributor Author

styfle commented Jul 21, 2017

@tianon @SimenB I updated the PR so the README.md is unchanged.
Hopefully this works as expected.

dog

@styfle
Copy link
Contributor Author

styfle commented Jul 25, 2017

The command ".travis/check-markdownfmt.sh" exited with 1.

What's wrong with the format this time?

@SimenB
Copy link

SimenB commented Jul 25, 2017

Can you run it locally?

@styfle
Copy link
Contributor Author

styfle commented Jul 25, 2017

@SimenB No, it doesn't seem to work with git bash on windows.

styfle@WINDOWS-PC MINGW64 /d/code/docker-docs (patch-1)
$ .travis/check-markdownfmt.sh
xargs: markdownfmt: No such file or directory

@SimenB
Copy link

SimenB commented Jul 25, 2017

Doing go get -u github.com/shurcooL/markdownfmt and ~/go/bin/markdownfmt -d node/license.md and ~/go/bin/markdownfmt -d node/content.md exits with 0, so I don't know what's wrong...

@SimenB
Copy link

SimenB commented Jul 26, 2017

I also tried ./markdownfmt.sh node/*.md (which uses a docker image) with no change

@tianon tianon merged commit 96e7f3e into docker-library:master Jul 31, 2017
@styfle styfle deleted the patch-1 branch August 1, 2017 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SimenB SimenB SimenB left review comments

@tianon tianon tianon approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@styfle @tianon @SimenB