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

Add GitInfo #2291

Merged
merged 1 commit into from Nov 1, 2016

Conversation

Projects
None yet
3 participants
@bep
Member

bep commented Jul 19, 2016

This commit adds a GitInfo object to Page if EnableGitInfo is set.

It then also sets Lastmod for the given Page to the author date provided by Git.

The Git integrations should be fairly performant, but it adds "some time" to the build, somewhat depending on the Git history size.

If you want, you can run without during development and turn it on when deploying to the live server: hugo --enableGitInfo.

Fixes #2102

@bep

This comment has been minimized.

Show comment
Hide comment
@bep

bep Jul 19, 2016

Member

This should work:

git -C /Users/bep/go/src/github.com/spf13/hugo/ log --name-only --format=format:"%t|%aN|%aE|%ai|%s" HEAD
Member

bep commented Jul 19, 2016

This should work:

git -C /Users/bep/go/src/github.com/spf13/hugo/ log --name-only --format=format:"%t|%aN|%aE|%ai|%s" HEAD
@bep

This comment has been minimized.

Show comment
Hide comment
@bep

bep Jul 20, 2016

Member

I have updated the PR. There are still some small holes, but it is complete enough to know that it is doable.

So if other could chime in with their opinions about:

  1. Is this a good idea? (I know I want it)
  2. Does this implementation make sense?
  3. Does setting Lastmod make sense?

/cc @spf13

Member

bep commented Jul 20, 2016

I have updated the PR. There are still some small holes, but it is complete enough to know that it is doable.

So if other could chime in with their opinions about:

  1. Is this a good idea? (I know I want it)
  2. Does this implementation make sense?
  3. Does setting Lastmod make sense?

/cc @spf13

@moorereason

This comment has been minimized.

Show comment
Hide comment
@moorereason

moorereason Jul 25, 2016

Contributor

Haven't tested this, but I like the idea. 👍

Is the AbbreviatedHash necessary? That could be derived later from the Hash if you needed it.

Contributor

moorereason commented Jul 25, 2016

Haven't tested this, but I like the idea. 👍

Is the AbbreviatedHash necessary? That could be derived later from the Hash if you needed it.

@bep

This comment has been minimized.

Show comment
Hide comment
@bep

bep Jul 25, 2016

Member

Is the AbbreviatedHash necessary?

You get it for free, and that is the value I want to show on my pages (if any); it's not possible to derive it from the Hash from inside of a Hugo template.

Member

bep commented Jul 25, 2016

Is the AbbreviatedHash necessary?

You get it for free, and that is the value I want to show on my pages (if any); it's not possible to derive it from the Hash from inside of a Hugo template.

@moorereason

This comment has been minimized.

Show comment
Hide comment
@moorereason

moorereason Jul 25, 2016

Contributor
{{ substr .GitInfo.Hash 0 -7 }}
Contributor

moorereason commented Jul 25, 2016

{{ substr .GitInfo.Hash 0 -7 }}
@bep

This comment has been minimized.

Show comment
Hide comment
@bep

bep Jul 25, 2016

Member

{{ substr .GitInfo.Hash 0 -7 }}

Which assumes 7 is a constant value.

Member

bep commented Jul 25, 2016

{{ substr .GitInfo.Hash 0 -7 }}

Which assumes 7 is a constant value.

@moorereason

This comment has been minimized.

Show comment
Hide comment
@moorereason

moorereason Jul 25, 2016

Contributor

My understanding was that it's configurable (via the core.abbrev setting). Defaults to 7. If you change the setting in your repo, you could update your template to match.

I just thought we could save memory allocs, but it's not that much of a win to drop it. You win. :)

Contributor

moorereason commented Jul 25, 2016

My understanding was that it's configurable (via the core.abbrev setting). Defaults to 7. If you change the setting in your repo, you could update your template to match.

I just thought we could save memory allocs, but it's not that much of a win to drop it. You win. :)

@bep

This comment has been minimized.

Show comment
Hide comment
@bep

bep Jul 25, 2016

Member

You kind of got a point, but it is simplest to let Git and the Git config win ... The memory loss is ... not much. Maybe Git-Junio will invent some smart "sliding abbreviated" hashes in the future ... Which is only length 3 for small repos ...

Member

bep commented Jul 25, 2016

You kind of got a point, but it is simplest to let Git and the Git config win ... The memory loss is ... not much. Maybe Git-Junio will invent some smart "sliding abbreviated" hashes in the future ... Which is only length 3 for small repos ...

@bep

This comment has been minimized.

Show comment
Hide comment
@bep

bep Oct 24, 2016

Member

Note to self: Maybe add a CLI flag for this as well, to make it easier to switch it off when in server mode etc.

Member

bep commented Oct 24, 2016

Note to self: Maybe add a CLI flag for this as well, to make it easier to switch it off when in server mode etc.

Add GitInfo
This commit adds a `GitInfo` object to `Page` if `EnableGitInfo` is set.

It then also sets `Lastmod` for the given `Page` to the author date provided by Git.

The Git integrations should be fairly performant, but it adds "some time" to the build, somewhat depending on the Git history size.

If you want, you can run without during development and turn it on when deploying to the live server: `hugo --enableGitInfo`.

Fixes #2102

@bep bep changed the title from Work in progress: Add GitInfo to Add GitInfo Oct 29, 2016

@bep bep added the NeedsReview label Oct 29, 2016

@bep

This comment has been minimized.

Show comment
Hide comment
@bep

bep Oct 29, 2016

Member

This is ready now. It needs some docs, but that will have to wait.

/cc @digitalcraftsman @moorereason @spf13 and gang.

Member

bep commented Oct 29, 2016

This is ready now. It needs some docs, but that will have to wait.

/cc @digitalcraftsman @moorereason @spf13 and gang.

@bep bep merged commit e8380e6 into gohugoio:master Nov 1, 2016

4 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/wercker Wercker build passed
Details
licence/cla Contributor License Agreement is signed.
Details

@bep bep deleted the bep:gitinfo branch Apr 18, 2017

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