Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v2: Implement git module #3171

Closed
vrongmeal opened this issue Mar 22, 2020 · 9 comments
Closed

v2: Implement git module #3171

vrongmeal opened this issue Mar 22, 2020 · 9 comments
Assignees
Labels
plugin 🔌 A feature outside this repo

Comments

@vrongmeal
Copy link
Contributor

Creating this issue to track progress for git module similar to the v1 git plugin.

@vrongmeal vrongmeal added the v2 label Mar 22, 2020
@vrongmeal vrongmeal self-assigned this Mar 22, 2020
@vrongmeal
Copy link
Contributor Author

vrongmeal commented Mar 22, 2020

Currently implementing the module using the git bindings for go.

  • Basic implementation of service by fetching at regular intervals.
  • Webhook implementation to fetch as soon as changes are detected.

@mholt mholt added the plugin 🔌 A feature outside this repo label Mar 22, 2020
@mholt
Copy link
Member

mholt commented Mar 22, 2020

Something to consider: src-d/go-git#1295

Perhaps the hard fork is a better dependency for now: https://github.com/go-git/go-git

Also, to clarify, this plugin will be hosted in a separate repository. You're welcome to track it here until your repo is pushed, though, I guess.

Thanks for working on this! Have fun with it!

@vrongmeal
Copy link
Contributor Author

There are a few things I wanted to discuss. The following is the struct that implements the module:

type Git struct {
	// URL of the git repository.
	Repo string `json:"repo"`

	// Path to clone the repository in.
	//
	// Defaults to site root. Can be absolute or relative to site root.
	Path string `json:"path,omitempty"`

	// Branch or tag of the repository to clone.
	//
	// Defaults to master. `{latest}` can be used as a
	// placeholder for latest tag which ensures the most recent tag is
	// always pulled.
	Branch string `json:"branch,omitempty"`

	// Git access token required for private repositories.
	Token string `json:"token,omitempty"`

	// Depth of commits to fetch.
	Depth int `json:"depth,omitempty"`

	// Interval is the number of seconds between pulls.
	//
	// Defaults to 3600 (1 hour). Minimum interval is 5. An interval
	// of -1 disables periodic pull.
	Interval int `json:"interval,omitempty"`
}

Can we rename branch to something like tree?

Secondly, any other suggestions in this? The webhook part I'll implement later on. Also, I've added access token option for private repos. I'll also add the option for fetching using private key. (IMO using access tokens is a better way)

@mholt
Copy link
Member

mholt commented Mar 22, 2020

Great start!

Can we rename branch to something like tree?

Yes! That's a better name IMO.

Secondly, any other suggestions in this?

So far:

  • // URL of the git repository. -- does this have to be the HTTPS form or can it also be SSH? Would be good to decide (or support both) and clarify. (Preferably both if it's not too hard.)
  • {latest} can be used as a placeholder -- before this plugin is stable, we should decide on a properly-namespaced placeholder, like {git.repo.latest} or something.
  • // Interval is the number of seconds between pulls. -- make this a caddy.Duration value, rather than requiring it to be seconds.
  • // Depth of commits to fetch. -- under what circumstances? All pulls? The v1 plugin has clone_args and pull_args which seem useful, have you considered that instead, maybe more flexible that way? (I dunno myself, I've never needed them.)
  • // Git access token required for private repositories. - this is fine, but before tagging a stable release you'll want to decide if you want to use the same field to specify a key file and, if so, maybe name it differently.

@tobya
Copy link
Collaborator

tobya commented Mar 22, 2020

ping @abiosoft

@francislavoie
Copy link
Member

Can we rename branch to something like tree?

Hmm, what's the reasoning for this? IMO branch feels more natural to me, even if it can also be a tag.

{latest} can be used as a placeholder -- before this plugin is stable, we should decide on a properly-namespaced placeholder, like {git.repo.latest} or something.

@mholt pretty sure this isn't the same kind of placeholder in the Caddy sense, it's just an internal magic value if I'm reading correctly.

@mholt
Copy link
Member

mholt commented Mar 22, 2020

Hmm, what's the reasoning for this? IMO branch feels more natural to me, even if it can also be a tag.

I think "tree" is more technically accurate, and makes it a little clearer that you're not limited to just branch names. I'm not particularly opinionated about the name of that field tbh.

pretty sure this isn't the same kind of placeholder in the Caddy sense, it's just an internal magic value if I'm reading correctly.

I'm not so sure, it seems like something exposed to the user.

@mholt mholt removed the v2 label Mar 23, 2020
@awoodbeck
Copy link

Be aware of the edge case presented by relative SSH URLs if you rely on url.Parse to validate the Repo field.

@mholt
Copy link
Member

mholt commented Apr 17, 2020

Closing, since this is now being tracked at https://github.com/vrongmeal/caddygit

@mholt mholt closed this as completed Apr 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugin 🔌 A feature outside this repo
Projects
None yet
Development

No branches or pull requests

5 participants