Git Product home page Git Product logo

caddygit's Introduction

caddygit's People

Contributors

vrongmeal avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

caddygit's Issues

Documentation for the Webhook service

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?

Fails to build on windows

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

Caddyfile

Hi!

I think may be interesting make a Caddyfile example (of this module) for caddy v2. Thanks for doing this module ๐Ÿ˜„

Initial feedback

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:

  • URL: Does it support SSH as well? (Is that something that can be easily added later?)
  • Path: Does it create a subfolder or does it go directly into the given path exactly as specified? What if the folder already exists and is not a git repo, etc? What is the default path?
  • Tag: The {latest} placeholder is a good idea! But it should be properly namespaced, and maybe a little more descriptive, like {git.repo.latest} for example.
  • Branch: Is it needful to have both 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.)
  • Username and Password: As much as I like having all the config in a single document, I think there should be a way for credentials to be specified outside the config file, like in an env variable.
  • SingleBranch and Depth: What are the defaults? What values are allowed? Would it make sense to keep clones/pulls lightweight by default and only clone the specified branch at a depth of 1? (Or would that break most sites?)
  • Interval: It feels weird to specify -1 to disable periodic pulling IMO, what if 0 disabled it instead? Actually... what is the point of disabling it? Wouldn't it be kind of useless then?
  • Then and ThenLong: I think the design of this feature could use some careful thought. Do users need to execute more than one command? If so, can we do something more like:
	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.

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    ๐Ÿ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. ๐Ÿ“Š๐Ÿ“ˆ๐ŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google โค๏ธ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.