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

Send more VCS details to lolsrv #181

Merged
merged 4 commits into from
Jan 1, 2014
Merged

Conversation

drewwells
Copy link
Contributor

This data is useful for making lolsrv more repository agnostic.

Additional information includes:

  • full repo details {user}/{repo}
  • Timestamps of files
  • Full URL of commit built from the commit remote path

These features are blocking eteubert/lolsrv#2

lolsrv more repository agnostic.

Additional information includes:

*  full repo details {user}/{repo}
*  Timestamps of files
*  Full URL of commit built from the commit remote path
@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) when pulling f7ca2fd on drewwells:master into 9d62c26 on mroth:master.

drewwells added a commit to drewwells/lolsrv that referenced this pull request Dec 6, 2013
Add shotgun and Guard for faster, better, stronger development
Consume NEW url and timestamp fields from plugin lolcommits/lolcommits#181
Rely on timestamp data when Github API call is not successful
Github API auth
Turned on Sintra config magic
Switched to modular Sintra for less config loading overhead
Google API keys read from environment, when available (reads from heroku when deployed)

Style updates and JS optimizations via lazy loading
Specify directly what size the images are
drewwells added a commit to drewwells/lolsrv that referenced this pull request Dec 6, 2013
Add shotgun and Guard for faster, better, stronger development
Consume NEW url and timestamp fields from plugin lolcommits/lolcommits#181
Rely on timestamp data when Github API call is not successful
Github API auth
Turned on Sintra config magic
Switched to modular Sintra for less config loading overhead
Google API keys read from environment, when available (reads from heroku when deployed)

Style updates and JS optimizations via lazy loading
Specify directly what size the images are
Prefer time field, it's more often available
@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) when pulling 81068c9 on drewwells:master into 9d62c26 on mroth:master.

@@ -13,7 +13,9 @@ def initialize
self.message = commit.message.split("\n").first
self.sha = commit.sha[0..10]
self.repo_internal_path = g.repo.path
regex = /.*[:\/]([\w\-]*).git/
self.url = g.remote.url ? g.remote.url.gsub(':','/').gsub(/^git@/,'https://').gsub(/\.git$/,'') + '/commit/': ''
Copy link
Member

Choose a reason for hiding this comment

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

seems like a rather lengthy expression here, maybe move this to a separate (private) method in this class? E.g.

def remote_https_url(url)
  url.gsub(':','/').gsub(/^git@/,'https://').gsub(/\.git$/,'') + '/commit/'  
end

# call with this (i think having self.url == nil is probably OK)
self.url = remote_https_url(g.remote.url) if g.remote.url

@matthutchinson
Copy link
Member

👍 Looks great, (just one minor comment) otherwise I think it's merge-able.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.22%) when pulling 10bf200 on drewwells:master into 9d62c26 on mroth:master.

@drewwells
Copy link
Contributor Author

Anything holding this up now?

:repo => self.runner.repo,
:date => File.ctime(file),
:sha => sha)
rescue => error
Copy link
Member

Choose a reason for hiding this comment

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

this should be rescue => e

@matthutchinson
Copy link
Member

couple more minor comments, otherwise good to go I think 👍

@coveralls
Copy link

Coverage Status

Coverage increased (+0.22%) when pulling 5dbe263 on drewwells:master into 9d62c26 on mroth:master.

@matthutchinson
Copy link
Member

Thanks, looks OK to me now, merging 👍

matthutchinson added a commit that referenced this pull request Jan 1, 2014
Send more VCS details to lolsrv
@matthutchinson matthutchinson merged commit 2878db1 into lolcommits:master Jan 1, 2014
@matthutchinson
Copy link
Member

released just now, in v0.5.3

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.

None yet

3 participants