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

Gitlw/migrate increment #2955

Merged
merged 3 commits into from
Jan 31, 2019
Merged

Gitlw/migrate increment #2955

merged 3 commits into from
Jan 31, 2019

Conversation

gitlw
Copy link

@gitlw gitlw commented Jan 30, 2019

This change is Reviewable

@gitlw gitlw requested review from manishrjain and a team January 31, 2019 00:02
Copy link
Contributor

@manishrjain manishrjain left a comment

Choose a reason for hiding this comment

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

Few comments.

Reviewed 4 of 4 files at r1.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @gitlw)


dgraph/cmd/root.go, line 70 at r1 (raw file):

var subcommands = []*x.SubCommand{
	&bulk.Bulk, &cert.Cert, &conv.Conv, &live.Live, &alpha.Alpha, &zero.Zero, &version.Version,
	&debug.Debug, &increment.Increment,

Call the package counter. So, it would be counter.Increment.


dgraph/cmd/increment/increment.go, line 20 at r1 (raw file):

// successful, it would print out the incremented value. It assumes that it has
// access to UID=0x01, and that `val` predicate is of type int.
package increment

counter


dgraph/cmd/increment/increment.go, line 42 at r1 (raw file):

	Increment.Cmd = &cobra.Command{
		Use:   "increment",
		Short: "increment a counter transactionally",

Capital i.


dgraph/cmd/increment/increment.go, line 52 at r1 (raw file):

	flag.Int("num", 1, "How many times to run.")
	flag.Bool("ro", false, "Only read the counter value, don't update it.")
	flag.Duration("wait", 0*time.Second, "How long to wait.")

This has to be a string, no? So, we can pass wait=1s?

Copy link
Author

@gitlw gitlw left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 4 files reviewed, 4 unresolved discussions (waiting on @manishrjain)


dgraph/cmd/root.go, line 70 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Call the package counter. So, it would be counter.Increment.

Done.


dgraph/cmd/increment/increment.go, line 20 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

counter

Done.


dgraph/cmd/increment/increment.go, line 42 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Capital i.

Done.


dgraph/cmd/increment/increment.go, line 52 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

This has to be a string, no? So, we can pass wait=1s?

Yes, we can do --wait=1s if it's a Duration.

Copy link
Contributor

@manishrjain manishrjain left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 4 of 4 files at r2.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@gitlw gitlw merged commit 20f2428 into master Jan 31, 2019
@gitlw gitlw deleted the gitlw/migrate_increment branch January 31, 2019 01:29
danielmai pushed a commit that referenced this pull request Feb 6, 2019
dna2github pushed a commit to dna2fork/dgraph that referenced this pull request Jul 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants