Skip to content
This repository has been archived by the owner on Jan 26, 2018. It is now read-only.

First pr merge semver #27

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

jonbodner
Copy link

This pull request includes the basic functionality for automatic merging and versioning using semantic versioning.


re *regexp.Regexp
re *regexp.Regexp
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is gofmt run on the code? I would expect it to remove the spacing after re

@bradrydzewski
Copy link
Member

bradrydzewski commented Jul 6, 2016

I do question if auto-tagging is within the scope of LGTM. LGTM focuses on approvals and has a pretty narrow scope. Perform task X when all statuses are passing seems like a separate service, which could be used by people not using LGTM. It also seems like it could include more than just tagging, including deployments, etc. I could also see this as a CI plugin (ie for drone) where you have a plugin that analyzes comments for the merged commit to determine if it should be tagged.

I do think the auto-merge makes sense since, in a restricted environment, you could have maintainers that do not actually have the appropriate access to merge the pull request in which case auto-merge would be very helpful.

@jonbodner
Copy link
Author

Heh; quite a few comments. I'll try to get through them tonight or tomorrow.

One quick note: in our internal use of this fork, we've found auto-tagging pretty useful. Sure, it could be in a second bot, but it's nice to have merge and version happen at the same time.

Also, the code was go fmt'd, but individual commits might not have been. I'll fix this on the branch along with the other comments.

jonbodner added 3 commits July 9, 2016 22:14
…oved GetMaxExistingTag from remote API and put in GetTags instead. Added GetTags to API. Added GetMaxTag method to new TagList type.
@jonbodner
Copy link
Author

I've addressed several of the issues pointed out. I still need to write tests to cover the changes, but I wanted to get some feedback first. If you're going to be at GopherCon, we can meet up and talk some more about which changes you want to incorporate.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants