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

Footer: show both "commit" and "last_updated" (if available) #897

Merged
merged 1 commit into from Jul 23, 2020

Conversation

mgeier
Copy link
Contributor

@mgeier mgeier commented Apr 8, 2020

Closes #238.

@mgeier
Copy link
Contributor Author

mgeier commented May 6, 2020

Any comments on this?

Copy link
Member

@ericholscher ericholscher left a comment

Choose a reason for hiding this comment

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

This looks like a good fix 👍

@mgeier
Copy link
Contributor Author

mgeier commented Jul 7, 2020

What about merging this? Is there anything that still needs to be done?

@ericholscher
Copy link
Member

@agjohnson this look good to you? I'm 👍 on it.

@ericholscher
Copy link
Member

@mgeier sorry for the delay, we'll get it merged here soon, once Anthony is 👍 on it.

@agjohnson
Copy link
Collaborator

agjohnson commented Jul 20, 2020

So, the extension that @mgeier wrote is the information that I'd want to have in the footer -- that is, the last modified date is the last commit date of the source file.

However, under normal usage without the extension, the last modified date is the date of the build of the project[1] -- so this information would no longer be truthy. It's actually probably more of a negative change to users, as RTD rebuilds projects clean every build, so I believe that a file that has not changed in years would show "last modified $today" on output.

So, this PR likely has too many unwanted side effects as-is for all the standard RTD users, but is there a better way to make the two extensions integrate in a nicer fashion? If we could detect when this extension is in use, or when we have a last_updated_from_git configuration variable, or something similar, I'd love to have that information in the output instead of just the last commit id.

For an estimated time frame, we have some time off and large projects to contend with right now, so the next feature release of this theme is likely in august. We have a few new features we want to release then, but expect bandwidth to be limited more immediately.

1: https://github.com/sphinx-doc/sphinx/blob/e65021fb9b0286f373f01dc19a5777e5eed49576/sphinx/builders/html/__init__.py#L443

@mgeier
Copy link
Contributor Author

mgeier commented Jul 21, 2020

this PR likely has too many unwanted side effects as-is for all the standard RTD users

I don't think so.

The important point is that a user has to opt-in to displaying the "last changed" date anyway, by specifying html_last_updated_fmt, typically like this:

html_last_updated_fmt = ''

Therefore, "standard RTD users" will not see the date, neither before nor after this PR.

I believe that a file that has not changed in years would show "last modified $today" on output.

Yes, but only if the author has opted into displaying it.

is there a better way to make the two extensions integrate in a nicer fashion? If we could detect when this extension is in use

I guess that's possible, but it's strictly unnecessary.

or when we have a last_updated_from_git configuration variable, or something similar, I'd love to have that information in the output instead of just the last commit id.

This would be nice, but a user might as well just use my extension.

For an estimated time frame, we have some time off and large projects to contend with right now, so the next feature release of this theme is likely in august. We have a few new features we want to release then, but expect bandwidth to be limited more immediately.

OK, good to know!

@agjohnson
Copy link
Collaborator

The important point is that a user has to opt-in to displaying the "last changed" date anyway, by specifying html_last_updated_fmt

Ah! So this is a detail I didn't know. This changes my opinion then, as the user has all of the control over the display of the last_updated variable. This PR should be safe.

It still isn't ideal that the date will be the last project build date, but that isn't something we'd solve in this theme.

This would be nice, but a user might as well just use my extension.

Yup, I was describing some option your extension could surface so this and other themes could be aware of it. I don't think this is necessary now though, as we can expect last_updated will only be used when the user wants the block.

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.

last_updated gone in footer ?
4 participants