vrongmeal / caddygit Goto Github PK
View Code? Open in Web Editor NEWGit module for Caddy v2
Git module for Caddy v2
Great work on this plugin! It looks like a great start!
I haven't pulled it down and played with it yet, but I will soon. Here's a few comments based on my first pass of the source code. It's a lot, so don't worry about tackling it all at once! And it's not criticism, just some ideas/questions from my perspective. It looks like you've done great work!
Module name: git
could just as well be a git server, but this is a git client. I actually like how concise this name is and think that it's totally possible for the git
app to be both a client and a server. That said, the structure of the app should be designed so that there is room for a plausible future git server. (Or, if the client and the server will be separate apps, the names should reflect that. However, I don't think that'll be strictly necessary.) This might be as simple as moving all the client-related config into a sub-struct of the Git struct
type:
type Git struct {
Client *GitClient `json:"client,omitempty"
}
type GitClient struct {
Repositories []Repository `json:"repositories"`
wg *sync.WaitGroup
}
Struct name: Consider naming it App
-- I haven't been super consistent about this and it's not a big deal, but then it's less repetitive than caddygit.Git
-- instead, it'd be caddygit.App
which makes a little more sense and is more descriptive.
Module registration: This should be changed:
func init() {
if err := caddy.RegisterModule(Git{}); err != nil {
caddy.Log().Fatal(err.Error())
}
}
to this:
func init() {
caddy.RegisterModule(Git{})
}
to work with the latest commit -- last breaking change! ๐
Module info: Avoid intialization here. Do it in Provision()
instead.
Interface guards: Looks good!
forEach: What is the purpose of this exactly? Why not just use a regular for
loop?
Repository
struct:
{latest}
placeholder is a good idea! But it should be properly namespaced, and maybe a little more descriptive, like {git.repo.latest}
for example.Tag
and Branch
fields? Could there be just a single field to specify which commit/branch/tag to pull? (Related idea: In the future, users may want a way to specify a tag pattern, i.e. the latest tag that matches a pattern -- so keep that in mind. Don't have to implement that now, I'm just saying to consider it so there's room for that to be added later.) CommandsAfter []Command `json:"commands_after,omitempty"`
...
}
type Command struct {
// Args[0] is the command to run, Args[1:] is the arguments, just
// like exec.Command - best to not have to do any parsing, you
// won't need go-shellquote
Args []string `json:"command,omitempty"`
// Or "Async", I don't care
Background bool `json:"background,omitempty"`
// maybe an option to determine whether the command chain
// terminates if this command returns an error, but you
// might also wait until this is requested by a user
}
Struct tags: Always specify omitempty
so that config adapters don't produce unnecessary, empty keys, which clutters up the resulting config. This will become noticeable once this app is usable from the Caddyfile.
Context: Consider using the caddy.Context
that is passed in during Provision
- you can store it in your App struct if you need to; this context value gets cancelled when the app is stopping. It looks like you've already got cancellation figured out using channels and waitgroups and there's nothing wrong with that per-se, but if using the context makes things simpler, I'd say at least look into it.
Logging: Great to see that you're using the zap.Logger facilities provided by the core!
Running commands: This looks leaky - if the goal is to run the command wait for it to terminate OR for some other event (like a timeout or cancellation), then consider something more like this: https://github.com/caddyserver/xcaddy/blob/master/environment.go#L186-L220
That's all for now -- keep up the good work! Can't wait to start using this.
The README mentions a Webhook service twice, once in the line comment of the type: poll
configuration example, and once at the bottom in the list of to dos, marked as done.
How would an exemplary configuration for the Webhook service look like, and how would it be triggered, eventually involving a secret?
$ go build
# github.com/vrongmeal/caddygit
..\caddygit\commands.go:80:45: unknown field 'Setpgid' in struct literal of type syscall.SysProcAttr
..\caddygit\commands.go:113:10: undefined: syscall.Kill
These don't seem to work on Windows.
Hi!
I think may be interesting make a Caddyfile example (of this module) for caddy v2. Thanks for doing this module ๐
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.