public
/
mirror
11
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Refactor Go Merlin #1

Closed
a268wang wants to merge 8 commits from refactor into go
pull from: refactor
merge into: public:go
Conversation 1 Commits 8 Files Changed 21
a268wang commented 2021-12-15 18:58:08 -05:00

Changes made

  • wrote some tests
  • split the common package into config, logger, and sync
  • moved the unix socket things from merlin.go into its own package
  • made some adjustments (splitting methods, moving stuff) to make it easier to test

Note: at the moment the drone.io is failing because the of some time zone stuff

Todo

  • write unit tests for the sync package
  • test the program as a whole
#### Changes made - wrote some tests - split the common package into config, logger, and sync - moved the unix socket things from merlin.go into its own package - made some adjustments (splitting methods, moving stuff) to make it easier to test Note: at the moment the drone.io is failing because the of some time zone stuff #### Todo - write unit tests for the sync package - test the program as a whole
a268wang added 8 commits 2021-12-15 18:58:09 -05:00
continuous-integration/drone Build is passing Details
5049b40ad9 attempt to trigger drone.io
continuous-integration/drone Build is passing Details
ea292c82f1 trigger drone.io
continuous-integration/drone/pr Build is failing Details
continuous-integration/drone Build is failing Details
4a76506b35 update .drone.yml to actually run the tests
d278liu requested changes 2021-12-16 23:10:39 -05:00
d278liu left a comment
repo.issues.collaborator

lgtm except the things in sync.go

lgtm except the things in sync.go
merlin/sync/command.go
@ -0,0 +246,4 @@
return
}
if repo.PasswordFile != "" {
d278liu commented 2021-12-16 23:07:00 -05:00
repo.issues.collaborator

same here, some sync types already append the password file - and how the password file should be added to args depends on the type of sync, so probably would remove this too

same here, some sync types already append the password file - and how the password file should be added to args depends on the type of sync, so probably would remove this too
a268wang commented 2021-12-16 23:27:01 -05:00
repo.issues.poster

I agree with this, I'll remove it shortly

I agree with this, I'll remove it shortly
a268wang marked this conversation as resolved
merlin/sync/command.go
@ -0,0 +251,4 @@
args = append(args, "--password-file", filename)
}
args = append(args, buildRsyncHost(repo), localDir)
d278liu commented 2021-12-16 22:52:51 -05:00
repo.issues.collaborator

don't think you want to be appending rsync host and localDir here too. each sync type already appends it.

also different sync types have different ways of building the host (some needs to be the daemon style host and some needs to be a ssh host)

don't think you want to be appending rsync host and localDir here too. each sync type already appends it. also different sync types have different ways of building the host (some needs to be the daemon style host and some needs to be a ssh host)
a268wang commented 2021-12-16 23:19:20 -05:00
repo.issues.poster

Huh all this stuff was actually moved over from the startSync function

https://git.csclub.uwaterloo.ca/public/mirror/src/branch/go/merlin/common/sync.go#L256-L270

I'm thinking of keeping a modified version of part to make sure that startSync will immediately exit if merlin cannot mkdir the buildDownloadDir

Huh all this stuff was actually moved over from the startSync function https://git.csclub.uwaterloo.ca/public/mirror/src/branch/go/merlin/common/sync.go#L256-L270 I'm thinking of keeping a modified version of part to make sure that startSync will immediately exit if merlin cannot mkdir the buildDownloadDir
d278liu commented 2021-12-16 23:47:34 -05:00
repo.issues.collaborator

yeah my bad about the original sync.go i think i forgot to remove them

yeah my bad about the original sync.go i think i forgot to remove them
a268wang marked this conversation as resolved
a268wang commented 2021-12-17 00:06:04 -05:00
repo.issues.poster

Lol merge request broke after pushing a commit to refactor.

"This pull request is broken due to missing fork information."

Lol merge request broke after pushing a commit to refactor. "This pull request is broken due to missing fork information."
a268wang closed this pull request 2021-12-17 00:06:04 -05:00
Some checks failed
continuous-integration/drone/pr Build is failing
Details
continuous-integration/drone Build is failing
Details

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No reviewers
d278liu
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: public/mirror#1
No description provided.
Delete Branch "refactor"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?