Summary

Add support for Mercurial repositories

Review Request #9409 — Created Nov. 29, 2017 and submitted June 26, 2018, 4:17 p.m.

Information

Owner

brennie

Repository

rb-gateway

Branch

master

Bugs

Depends On

~~10026~~, ~~10009~~

Blocks

10021, 10024

Commit

974ed29...

Reviewers

Groups

rb-gateway, students

People

Description

RB Gateway now supports Mercurial repositories. Setting scm to hg
in a repository entry in the configuration file will enable this
behaviour.

Co-authored-by: Henri-Philippe Marceau hmarceau@laurentian.ca

Testing Done

Ran go fmt ./....
Ran go test ./....

Configured a Mercurial repository and made HTTP requests to verify
API responses were correct.

Issues

Description	From	Last Updated
This is a small nitpick but I've looked in a few other files, and I think there should be a …	RC rcreagha	Dec. 10, 2017, 12:12 p.m.
Not sure if this applies to go but this line is longer than 78 characters.	GR GrahamS	Dec. 10, 2017, 12:13 p.m.
I think this import list should be alphabetical.	RC rcreagha	Dec. 10, 2017, 12:18 p.m.
This line is also longer than 78 characters.	GR GrahamS	Dec. 10, 2017, 12:17 p.m.
Should be "On success" rather than "On a success"	david	June 11, 2018, 5:32 p.m.
Shouldn't this be returning false, err?	david	June 11, 2018, 5:35 p.m.
Shouldn't this be returning false, err?	david	June 11, 2018, 5:37 p.m.
This should have documentation.	david	June 18, 2018, 10:55 p.m.
Typo: repsonsible -> responsible	david	June 19, 2018, 12:04 p.m.
Can we say "Mercurial repository" instead of "Hg repository", so it's less of a tautology?	david	June 18, 2018, 10:55 p.m.

flake8 passed.

JSHint passed.

Description:

		Added the `hg` case to `LoadConfig`, this allows Hg repos to be defined
		in the config file. The `HgRepository` type has been created and
~		currently supports `GetName`, `GetPath`, `GetFile`, `GetFileByCommit`&
~		`GetBranches`. Interactions with Hg repos are done through Mercurial's
~		command server through gohg.
	~	currently supports `GetName`, `GetPath`, `GetFile`, `GetFileByCommit`,
	~	`GetBranches` & `GetCommits`. Interactions with Hg repos are done through
	~	Mercurial's command server through gohg.

Testing Done:

		`httpie` was used to send `GetName`, `GetPath`, `GetFile`,
~		`GetFileByCommit` & `GetBranches` GET requests requesting information
~		from an Hg repo. Each request yielded the expected result. A unit test
~		was also written and can be found in `hg_repository_test.go` under
~		`TestHgGetBranches`.
	~	`GetFileByCommit`, `GetBranches` & `GetCommits`GET requests requesting
	~	information from an Hg repo. Each request yielded the expected result. A
	~	unit test was also written and can be found in `hg_repository_test.go`
	~	under `TestHgGetBranches`.

Commit:

-	6fca13ed1dea43979f295641d071757b6907e33a
+	012ce9cc49d93ddf4400adc03c10265087db2982

Diff:

Revision 2 (+486)

Show changes

	hg_repository.go
	hg_repository_test.go
	util.go

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Summary:

-	[WIP]Adding support for Mercurial repositories to RB-Gateway.
+	Adding support for Mercurial repositories to RB-Gateway.

Description:

~		Added the `hg` case to `LoadConfig`, this allows Hg repos to be defined
~		in the config file. The `HgRepository` type has been created and
~		currently supports `GetName`, `GetPath`, `GetFile`, `GetFileByCommit`,
~		`GetBranches` & `GetCommits`. Interactions with Hg repos are done through
~		Mercurial's command server through gohg.
	~	`hg_repository.go`, a Mercurial specific implementation of `repository.go`
	~	was created. The `hg` case was added to `LoadConfig` in `util.go` to
	~	allow Mercurial repositories to be defined in the config file.
	~	`hg_repository_test.go` was created, it contains unit tests for every
	~	method implemented in `hg_repository.go`.

Testing Done:

		`httpie` was used to send `GetName`, `GetPath`, `GetFile`,
~		`GetFileByCommit`, `GetBranches` & `GetCommits`GET requests requesting
~		information from an Hg repo. Each request yielded the expected result. A
~		unit test was also written and can be found in `hg_repository_test.go`
~		under `TestHgGetBranches`.
	~	`GetFileByCommit`, `GetBranches`, `GetCommits` & `GetCommit` requests to
	~	a tester Hg repo. Each request yielded the expected result. Unit tests
	~	were also written and can be found in `hg_repository_test.go`. All of the
	~	test were run and all were passed. Also, Review Board successfully
	+	accessed a Mercurial Repository through RB-Gateway.

Commit:

-	012ce9cc49d93ddf4400adc03c10265087db2982
+	503541274300d195c08e04f59daab0c625d64d5c

Diff:

Revision 3 (+713)

Show changes

	hg_repository.go
	hg_repository_test.go
	util.go

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

I'll say upfront that I know nothing about Go, so all I can really comment on is a few stylistic things.

hg_repository.go (Diff revision 3)

Show all issues

This is a small nitpick but I've looked in a few other files, and I think there should be a space between 'import' and the parenthesis.
Also I think this import list should be alphabetical.

hmarceau Dec. 10, 2017, 12:12 p.m.
```
You are right, thanks!
```

hg_repository.go (Diff revision 3)

There are a few spots like this where a few more line breaks could make this a little nicer looking.
I've been comparing against git_repository.go and that file has linebreaks after most if-statements.
However, I don't understand Go, so there may be a valid reason to group it together in some places.

hg_repository.go (Diff revision 3)

This is another example of where I think an extra line would be good to have, just between the end of the if block and the last return statement

hg_repository_test.go (Diff revision 3)

Show all issues

I think this import list should be alphabetical.

All in all the styling is nice and the comments are good. Good work!

hg_repository.go (Diff revision 3)

Show all issues

Not sure if this applies to go but this line is longer than 78 characters.

hg_repository_test.go (Diff revision 3)
Show all issues
```
This line is also longer than 78 characters.
```

Commit:

-	503541274300d195c08e04f59daab0c625d64d5c
+	3d7e3b75fc222c369b0565ebc4aee98895671bc1

Diff:

Revision 4 (+712)

Show changes

	hg_repository.go
	hg_repository_test.go
	util.go

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

hg_repository.go (Diff revision 4)

I don't know if the multiple line-comments are due to a style-guide or anything, but if not I believe you can do C-style /* */ multi-line comments in Go!

Change Summary:

Re-assign to self for final cleanup.

Owner:

-	hmarceau
+	brennie

Depends On:

+	~~10007 - Reorganize helpers/ package in prep for HG support~~

Change Summary:

Clean up work by Henri-Philippe.

This is pending RB getting HG support for rb-gateway.

Summary:

-	Adding support for Mercurial repositories to RB-Gateway.
+	Add support for Mercurial repositories

Description:

~		`hg_repository.go`, a Mercurial specific implementation of `repository.go`
~		was created. The `hg` case was added to `LoadConfig` in `util.go` to
~		allow Mercurial repositories to be defined in the config file.
~		`hg_repository_test.go` was created, it contains unit tests for every
~		method implemented in `hg_repository.go`.
	~	RB Gateway now supports Mercurial repositories. Setting `scm` to `hg`
	~	in a repository entry in the configuration file will enable this
	~	behaviour.
	~
	~	Co-authored-by: Henri-Philippe Marceau hmarceau@laurentian.ca

Testing Done:

~		`httpie` was used to send `GetName`, `GetPath`, `GetFile`,
~		`GetFileByCommit`, `GetBranches`, `GetCommits` & `GetCommit` requests to
~		a tester Hg repo. Each request yielded the expected result. Unit tests
~		were also written and can be found in `hg_repository_test.go`. All of the
~		test were run and all were passed. Also, Review Board successfully
	~	Ran `go fmt ./...`.
	~	Ran `go test ./...`.
	~
	~	Configured a Mercurial repository and made HTTP requests to verify
	~	API responses were correct.
-		accessed a Mercurial Repository through RB-Gateway.

Depends On:

-	~~10007 - Reorganize helpers/ package in prep for HG support~~
+	~~10009 - Add types for branches, commits, and commit metadata~~

Commit:

-	3d7e3b75fc222c369b0565ebc4aee98895671bc1
+	70fa666365bbd9aec93cb6665735531bd126d47b

Diff:

Revision 5 (+584 -9)

Show changes

	Gopkg.toml
	Gopkg.lock
	config/config.go
	helpers/git.go
	helpers/hg.go
	repositories/hg.go
	repositories/hg_test.go

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

repositories/hg.go (Diff revision 5)

Show all issues

Should be "On success" rather than "On a success"

repositories/hg.go (Diff revision 5)
Show all issues
```
Shouldn't this be returning false, err?
```
repositories/hg.go (Diff revision 5)
Show all issues
```
Shouldn't this be returning false, err?
```

Change Summary:

Address issues

Commit:

-	70fa666365bbd9aec93cb6665735531bd126d47b
+	9817bc6f7f596bd58c073c9dc602329a482c1528

Diff:

Revision 6 (+597 -9)

Show changes

	Gopkg.toml
	Gopkg.lock
	config/config.go
	helpers/git.go
	helpers/hg.go
	repositories/hg.go
	repositories/hg_test.go

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:

go fmt ./...

Commit:

-	9817bc6f7f596bd58c073c9dc602329a482c1528
+	e22ad3f4eea0dfe7e99d2228585f3b9b4ffb0d81

Diff:

Revision 7 (+596 -9)

Show changes

	Gopkg.toml
	Gopkg.lock
	config/config.go
	helpers/git.go
	helpers/hg.go
	repositories/hg.go
	repositories/hg_test.go

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:

Split strings more nicely

Commit:

-	e22ad3f4eea0dfe7e99d2228585f3b9b4ffb0d81
+	77e4450e41064474ad8fdc7ec808d25e2709c532

Diff:

Revision 8 (+592 -9)

Show changes

	Gopkg.toml
	Gopkg.lock
	config/config.go
	helpers/git.go
	helpers/hg.go
	repositories/hg.go
	repositories/hg_test.go

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:

Rename CreateHgBookmark to SeedHgBookmark

Commit:

-	77e4450e41064474ad8fdc7ec808d25e2709c532
+	b4cef109ede01bd6b5d4ef7cc664d64f1ba89aec

Diff:

Revision 9 (+592 -9)

Show changes

	Gopkg.toml
	Gopkg.lock
	config/config.go
	helpers/git.go
	helpers/hg.go
	repositories/hg.go
	repositories/hg_test.go

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:

refactor

Commit:

-	b4cef109ede01bd6b5d4ef7cc664d64f1ba89aec
+	942e44c8860cff0e7f382b7391ed365eec1b0819

Diff:

Revision 10 (+626 -9)

Show changes

	Gopkg.toml
	Gopkg.lock
	config/config.go
	helpers/git.go
	helpers/hg.go
	repositories/hg.go
	repositories/hg_test.go

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

helpers/hg.go (Diff revision 10)
Show all issues
```
This should have documentation.
```

repositories/hg.go (Diff revision 10)

Show all issues

Can we say "Mercurial repository" instead of "Hg repository", so it's less of a tautology?

Change Summary:

Address issues

Commit:

-	942e44c8860cff0e7f382b7391ed365eec1b0819
+	07dc8a7be2a9399d77bda2f2dc20c873311d837f

Diff:

Revision 11 (+633 -9)

Show changes

	Gopkg.toml
	Gopkg.lock
	config/config.go
	helpers/git.go
	helpers/hg.go
	repositories/hg.go
	repositories/hg_test.go

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

helpers/hg.go (Diff revisions 10 - 11)
Show all issues
```
Typo: repsonsible -> responsible
```

Change Summary:

Address issues

Commit:

-	07dc8a7be2a9399d77bda2f2dc20c873311d837f
+	e47f975d6b4cbbdce13deef08771fb3a9ce0d0c3

Diff:

Revision 12 (+633 -9)

Show changes

	Gopkg.toml
	Gopkg.lock
	config/config.go
	helpers/git.go
	helpers/hg.go
	repositories/hg.go
	repositories/hg_test.go

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:

Rebase

Commit:

-	e47f975d6b4cbbdce13deef08771fb3a9ce0d0c3
+	962c914be14967551b3f6df60a4a8ef7ab1272e4

Diff:

Revision 13 (+635 -7)

Show changes

	Gopkg.toml
	Gopkg.lock
	config/config.go
	helpers/git.go
	helpers/hg.go
	repositories/hg.go
	repositories/hg_test.go

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Depends On:

+	~~10026 - Fix up warnings from `go vet`~~

Change Summary:

Fix go vet issue

Commit:

-	962c914be14967551b3f6df60a4a8ef7ab1272e4
+	974ed296bae168443c63f815014059b55eba5aaf

Diff:

Revision 14 (+635 -7)

Show changes

	Gopkg.toml
	Gopkg.lock
	config/config.go
	helpers/git.go
	helpers/hg.go
	repositories/hg.go
	repositories/hg_test.go

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to master (aefa014)