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

FR: Better handling of author dates (as opposed to committer dates) #2000

Closed
ilyagr opened this issue Aug 7, 2023 · 8 comments · Fixed by #3906
Closed

FR: Better handling of author dates (as opposed to committer dates) #2000

ilyagr opened this issue Aug 7, 2023 · 8 comments · Fixed by #3906
Labels
enhancement New feature or request polish🪒🐃 Make existing features more convenient and more consistent

Comments

@ilyagr
Copy link
Collaborator

ilyagr commented Aug 7, 2023

This is an attempt to summarize a Discord discussion started by @chriskrycho.

Is your feature request related to a problem? Please describe.

Currently, jj sets the author date of a commit on jj new and does not change it when the commit is rewritten. This matches the letter git's behavior, but can be surprising.

For example, one might do jj new main today, then come three weeks later and start working on something. The author date for that commit will be three weeks before any work was done on it.

Describe alternatives you've considered

Workaround: @yuja pointed out the workaround: a user can run jj describe --reset-author --no-edit at any time to update the author date manually.

We have two proposed changes to this behavior:

  1. @yuja suggested to record the author date when the commit first becomes non-"discardable". AFAIK, this means either non-empty, or with children, or with more than one parent.

    This would match the spirit of the current behavior: the author date would better represent when the work on a change was started.

    IMO, we could alternatively simply record the moment when the commit became non-empty, which might be a bit simpler and seems to make little difference.

  2. @arxanas explained that git-branchless updates the author date on every edit to the commit except rebasing of the commit.

    This seems very sensible to me: the author date would represent when the work on a commit was finished.

  3. (Rejected?) In git, people tend to commit when a change first works (e.g. compiles). We could aim for a design that tries to make the author date represent that, but it seems difficult since this is very ambiguous from jj's perspective.

Describe the solution you'd like

We have to decide whether we prefer behavior 1, or 2, or an option to switch between them. Having an option is currently my preference.

@ilyagr ilyagr added enhancement New feature or request polish🪒🐃 Make existing features more convenient and more consistent labels Aug 7, 2023
@necauqua
Copy link
Collaborator

necauqua commented Aug 8, 2023

Isn't 1 and 2 like literally the git author date and committer date?
Except "committer date" does not change on rebases which is actually sensible

Storing both "start of the work" and "end of the work" seems beneficial to me

@ilyagr
Copy link
Collaborator Author

ilyagr commented Aug 8, 2023

I believe Github changes the committer date when a PR gets merged. I'm guessing that's what the committer date is for: when was the change merged to the trunk?

@necauqua
Copy link
Collaborator

necauqua commented Aug 8, 2023

It does it because it doesn't fast-forward :(
And git changes it on rebases

@arxanas
Copy link
Collaborator

arxanas commented Aug 8, 2023

Isn't 1 and 2 like literally the git author date and committer date?
Except "committer date" does not change on rebases which is actually sensible

The first case seems like an author date, but I wouldn't call the second case a committer date. The committer date is updated on rebases (and the author date isn't), and it's a different thing than the "end of work" date.

@vwkd
Copy link
Collaborator

vwkd commented Nov 21, 2023

Not sure about 2. If I come back one week later to fix a tiny typo, it would update the author date which isn't what I'd want.

Maybe git's commit notion could actually be useful here. commit could set the author date without creating a new commit (unlike new). So, if you end end your day with commit (instead of new), and a week later correct a tiny typo before moving on to new work with new, it wouldn't change the author date of the previous work.

@ilyagr
Copy link
Collaborator Author

ilyagr commented Feb 10, 2024

#3008 suggested the option of setting the timestamp when the description becomes non-empty.

I think I'm leaning towards @yuja's suggestion of making either non-empty description or non-empty content fix the author date. His suggestion might also fix the author date whenever a commit becomes a merge commit; I'd prefer not doing that, but it doesn't really matter much.

@scott2000
Copy link
Collaborator

I think that @yuja's suggestion of fixing the author date when a commit becomes non-discardable makes the most sense. This seems to be the most conservative option, since it doesn't throw away information about the author timestamp in any cases where it might be desirable to keep it.

Since discardable commits (i.e those with no description, one parent, and no changes) are already automatically discarded in some cases, it makes sense to me that the author timestamp could be lost in this case as well, while I might not expect it to be lost in other cases.

It might be nice to add a --reset-author flag to jj commit as well though (and maybe even a config option to set it by default) to make it possible to replicate Git's behavior easily.

@joyously
Copy link

joyously commented Jun 1, 2024

Is there an intent to store two dates? Would it make sense to work like file dates (creation date, modified date)?
I think the date when it becomes non-empty could be useful as a quasi-creation date, but the date last modified makes sense for when the commit ID changes.
What is not taken into account here is specifying a date for folks like me that want to preserve history of stuff that isn't stored in a VCS (like a bunch of zip files). I think that would require an option like --commit-time.

scott2000 added a commit to scott2000/jj that referenced this issue Jun 17, 2024
It's common to create empty working-copy commits while using jj, and
currently the author timestamp for a commit is only set when it is first
created. If you create an empty commit, then don't work on a repo for a
few days, and then start working on a new feature without abandoning the
working-copy commit, the author timestamp will remain as the time the
commit was created rather than being updated to the time that work began
or finished.

This commit adds a new revset `incomplete()` which specifies which
commits are "incomplete" and should have their author timestamp updated
whenever changes are made to them. This can be configured to fit many
different workflows. By default, empty commits with no description are
considered incomplete, meaning that the author timestamp will become
finalized whenever a commit is given a description or becomes non-empty.

The author timestamp is only updated if the committer is the author, and
it is never updated when rebasing.

Resolves jj-vcs#2000.
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Jul 7, 2024
## [0.19.0] - 2024-07-03

### Breaking changes

* In revset aliases, top-level `kind:pattern` expression is now parsed as
  modifier. Surround with parentheses if it should be parsed as string/file
  pattern.

* Dropped support for automatic upgrade of repo formats used by versions before
  0.12.0.

* `jj fix` now defaults to the broader revset `-s reachable(@, mutable())`
  instead of `-s @`.

* Dropped support for deprecated `jj branch delete`/`forget` `--glob` option.

* `jj branch set` now creates new branch if it doesn't exist. Use `jj branch
  move` to ensure that the target branch already exists.
  [#3584](jj-vcs/jj#3584)

### Deprecations

* Replacing `-l` shorthand for `--limit` with `-n` in `jj log`, `jj op log`
  and `jj obslog`.

* `jj split --siblings` is deprecated in favor of `jj split --parallel` (to
  match `jj parallelize`).

* A new `jj file` subcommand now replaces several existing uncategorized
  commands, which are deprecated.
  - `jj file show` replaces `jj cat`.
  - `jj file chmod` replaces `jj chmod`.
  - `jj file list` replaces `jj files`.

### New features

* Support background filesystem monitoring via watchman triggers enabled with
  the `core.watchman.register_snapshot_trigger = true` config.

* Show paths to config files when configuration errors occur.

* `jj fix` now supports configuring the default revset for `-s` using the
  `revsets.fix` config.

* The `descendants()` revset function now accepts an optional `depth` argument;
  like the `ancestors()` depth argument, it limits the depth of the set.

* Revset/template aliases now support function overloading.
  [#2966](jj-vcs/jj#2966)

* Conflicted files are individually simplified before being materialized.

* The `jj file` subcommand now contains several existing file utilities.
  - `jj file show`, replacing `jj cat`.
  - `jj file chmod` replacing `jj chmod`.
  - `jj file list` replacing `jj files`.

* New command `jj branch move` let you update branches by name pattern or source
  revision.

* New diff option `jj diff --name-only` allows for easier shell scripting.

* In color-words diffs, hunks are now highlighted with underline. See [diff
  colors and styles](docs/config.md#diff-colors-and-styles) for customization.

* `jj git push -c <arg>` can now accept revsets that resolve to multiple
  revisions. This means that `jj git push -c xyz -c abc` is now equivalent to
  `jj git push -c 'all:(xyz | abc)'`.

* `jj prev` and `jj next` have gained a `--conflict` flag which moves you
  to the next conflict in a child commit.

* New command `jj git remote set-url` that sets the url of a git remote.

* Author timestamp is now reset when rewriting discardable commits (empty
  commits with no description) if authored by the current user.
  [#2000](jj-vcs/jj#2000)

* `jj commit` now accepts `--reset-author` option to match `jj describe`.

* `jj squash` now accepts a `--keep-emptied` option to keep the source commit.

### Fixed bugs

* `jj git push` now ignores immutable commits when checking whether a
  to-be-pushed commit has conflicts, or has no description / committer / author
  set. [#3029](jj-vcs/jj#3029)

* `jj` will look for divergent changes outside the short prefix set even if it
  finds the change id inside the short prefix set.
  [#2476](jj-vcs/jj#2476)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request polish🪒🐃 Make existing features more convenient and more consistent
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants