yanyi / generate-multifields Goto Github PK

Overview

Following PRs #13 and #14, the GitHub badge in README isn't changed to reflect the master branch:

Overview

I was thinking about a release strategy. After going through and making PRs #13 and #14, I looked around because I had wanted master to only be the branch with all the changes. Away from the GitFlow strategy.

Browsing the Docker CE's README.md at 19.03 tag, I noted a few pointers:

1. When a release is started for Docker CE, a new branch will be created from master. Branch names will be YY.MM to represent the time-based release version of the product, e.g. 17.06.
2. There will be a git tag for each release candidate (RC) and general availability (GA) release. The tag will only point to commits on release branches.

This issue serves as an open document of coming up with a strategy to release the CLI. The release should be automated to:

1. Push to Docker Hub with a version tag, e.g. yanyi/generate-multifields:0.1.0.
2. Build the CLI with Go for different platforms and then uploading them to GitHub's Releases.

Overview

Include a CONTRIBUTING.md. Serves as a purpose to maintain the repository in a good state, too!

Here are some important points plucked from Docker CLI's CONTRIBUTING.md:

Branch Names

Fork the repository and make changes on your fork in a feature branch:

• If it's a bug fix branch, name it XXXX-something where XXXX is the number of the issue.
• If it's a feature branch, create an enhancement issue to announce your intentions, and name it XXXX-something where XXXX is the number of the issue.

I like this idea!

Unit Tests and Documentation

Submit unit tests for your changes. Go has a great test framework built in; use it! Take a look at existing tests for inspiration. Run the full test suite on your branch before submitting a pull request.

Update the documentation when creating or modifying features. Test your documentation changes for clarity, concision, and correctness, as well as a clean documentation build. See our contributors guide for our style guide and instructions on building the documentation.

Tests

For running tests, the CI runs it automatically on every push. However, it is still great that a dev can run tests before pushing or merging (test-driven development).

Currently, there's no code coverage number reporting. Could be a small implementation to improve the CI process.

Documentation

Tests could serve as documentation for people browsing the repository. Currently, the code base is small enough to not require extensive documentation. Follow idiomatic Go of writing comments for exported functions/methods. Make use of test examples or table-drive tests as part of documentation to understand what a certain function/method does.

Code

Write clean code. Universally formatted code promotes ease of writing, reading, and maintenance. Always run gofmt -s -w file.go on each changed file before committing your changes. Most editors have plug-ins that do this automatically.

Most IDEs should be doing this for you, especially VS Code with the Go extension. Could be an improvement to lint and reject commits during CI if it is not formatted properly, instead of formatting for the committer automatically.

Pull Requests and Commit Messages

Pull Requests

Pull request descriptions should be as clear as possible and include a reference to all the issues that they address.

Basics.

Code review comments may be added to your pull request. Discuss, then make the suggested modifications and push additional commits to your feature branch. Post a comment after pushing. New commits show up in the pull request automatically, but the reviewers are notified only when you comment.

Pull requests must be cleanly rebased on top of master without multiple branches mixed into the PR.

Might want to add instructions for people who are not that familiar with Git. Basically:

1. git checkout master
2. git pull --rebase (while on master)
3. git checkout - (to go back to your branch)
4. git rebase master

Afterwards, solve the conflicts (if there are). Then git push --force-with-lease in order to update your branch on remote.

Git tip: If your PR no longer merges cleanly, use rebase master in your feature branch to update your pull request rather than merge master.

Keep the git log branch lines clean.

Commit Messages

Commit messages must start with a capitalized and short summary (max. 50 chars) written in the imperative, followed by an optional, more detailed explanatory text which is separated from the summary by an empty line.

I favour this.

Overview

The indentation and alignment of the text looks out of place when running the help command:

$ generate-multifields help
generate-multifields is a CLI library to help generate multiple
	fields of a given input format of your GraphQL queries or mutations, by
	repeating them for a number of times.

It should look like:

$ generate-multifields help
generate-multifields is a CLI library to help generate multiple
fields of a given input format of your GraphQL queries or mutations, by
repeating them for a number of times.

How to Replicate

1. Run generate-multifields help.

Solution

This is due to the multiline string in Go, and go fmt probably added the tabs automatically. This was introduced in this commit: 8c7595f#diff-ff7686b39bf90dc2520886fb874371a4.

Since it is a multiline string, just removing the tabs like the following will do:

var (
	rootCmd = &cobra.Command{
		Long: `generate-multifields is a CLI library to help generate multiple
fields of a given input format of your GraphQL queries or mutations, by
repeating them for a number of times.`,
	}
)

Overview

Suggestion: add an alias instruction in the README for:

docker run --rm -v $(pwd):/tmp yanyi/generate-multifields:latest

Maybe running like:

gmf mutations -s 10 -e 15 /tmp/replace.txt

Overview

According to the Filesystem Hierarchy Standard (FHS):

/usr is the second major section of the filesystem. /usr is shareable, read-only data. That means that /usr should be shareable between various FHS-compliant hosts and must not be written to. Any information that is host-specific or varies with time is stored elsewhere.

Large software packages must not use a direct subdirectory under the /usr hierarchy.

Solution

Consider changing to recommend using /tmp instead:

The /tmp directory must be made available for programs that require temporary files.

Programs must not assume that any files or directories in /tmp are preserved between invocations of the program.

Or modify the program to read from stdin like:

cat hero.txt | generate-multifields -s 10 -e 15 -f -
# OR
cat hero.txt | generate-multifields -s 10 -e 15 -