Skip to content

Conversation

@iloveitaly
Copy link
Contributor

No description provided.

Copy link
Member

Choose a reason for hiding this comment

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

The ENV change is good, but otherwise this assumes too much (version could be mercurial or something unrelated to VCS). Mind just removing this bit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the app isn't versioned with git, git rev-parse --short HEAD.strip` will gracefully fallback to an empty string. I can go ahead and remove this, but keeping it in place might be a great added feature for devs with no harmful failure case. Thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

It's also the fact that git may not even be available on the system -- unless you're saying thats safe (not a rubyist myself)

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed on needing a git fallback, 99% of Ruby devs use it but perhaps not guaranteed in production environments.

@iloveitaly iloveitaly force-pushed the auto-release-tagging branch from ba1a262 to 3bff897 Compare October 15, 2015 23:36
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call on git not existing on the system. I wrapped the sys call in a rescue: this will fail gracefully if the user doesn't have git installed, or if the current directory is not a git repo.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess in that case release will be nil. Anything else we can fall back to? I guess not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, that's right: if the user isn't on heroku, and isn't using git, then release == nil. Although not idea, I think this solution will auto-tag errors with commit in the majority of cases.

@seanlinsley
Copy link
Contributor

It'd also be good to grab the release info from Capistrano deploys. This is how we're doing it:

config.release = File.read('REVISION').strip if File.exists? 'REVISION'

* heroku
* git repo
* capistrano version
@iloveitaly iloveitaly force-pushed the auto-release-tagging branch from 3bff897 to 5bd30ff Compare October 16, 2015 15:52
@iloveitaly
Copy link
Contributor Author

It'd also be good to grab the release info from Capistrano deploys

Good idea. Added this to the PR.

@dcramer
Copy link
Member

dcramer commented Oct 16, 2015

lgtm unless anyone has any objections

nateberkopec added a commit that referenced this pull request Oct 16, 2015
Attempt to pull release version from heroku, then from git repo
@nateberkopec nateberkopec merged commit 6b52f7d into getsentry:master Oct 16, 2015
@nateberkopec
Copy link
Contributor

Need to fix the dang build

@seanlinsley
Copy link
Contributor

Gem::InstallError: rack-cache requires Ruby version >= 1.9.3.

@nateberkopec
Copy link
Contributor

Yeah, PR's welcome to fix that part of the gem spec

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants