Add support for Mercurial repositories

Review Request #9409 — Created Nov. 29, 2017 and updated

brennie
rb-gateway
master
10026, 10009
10024, 10021
974ed29...
rb-gateway, students

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

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

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

  • 0
  • 0
  • 9
  • 1
  • 10
Description From Last Updated
hmarceau
hmarceau
  1. I'll say upfront that I know nothing about Go, so all I can really comment on is a few stylistic things.

  2. hg_repository.go (Diff revision 3)
     
     
     
     
     
     
     
     

    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.

    1. You are right, thanks!

  3. 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.

  4. 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

  5. hg_repository_test.go (Diff revision 3)
     
     
     
     
     
     
     
     
     

    I think this import list should be alphabetical.

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

  1. 
      
  2. hg_repository.go (Diff revision 3)
     
     

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

  3. hg_repository_test.go (Diff revision 3)
     
     

    This line is also longer than 78 characters.

  4. 
      
hmarceau
  1. 
      
  2. 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!
  3. 
      
brennie
brennie
david
  1. 
      
  2. repositories/hg.go (Diff revision 5)
     
     

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

  3. repositories/hg.go (Diff revision 5)
     
     

    Shouldn't this be returning false, err?

  4. repositories/hg.go (Diff revision 5)
     
     

    Shouldn't this be returning false, err?

  5. 
      
brennie
brennie
brennie
brennie
brennie
david
  1. 
      
  2. helpers/hg.go (Diff revision 10)
     
     

    This should have documentation.

  3. repositories/hg.go (Diff revision 10)
     
     

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

  4. 
      
brennie
david
  1. 
      
  2. helpers/hg.go (Diff revisions 10 - 11)
     
     

    Typo: repsonsible -> responsible

  3. 
      
brennie
brennie
brennie
brennie
Review request changed

Change Summary:

Fix go vet issue

Commit:

-962c914be14967551b3f6df60a4a8ef7ab1272e4
+974ed296bae168443c63f815014059b55eba5aaf

Diff:

Revision 14 (+635 -7)

Show changes

Checks run (2 succeeded)

flake8 passed.
JSHint passed.
Loading...