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

Should we use committer date instead of author date? #16

Open
gavanderhoorn opened this issue Aug 24, 2020 · 2 comments
Open

Should we use committer date instead of author date? #16

gavanderhoorn opened this issue Aug 24, 2020 · 2 comments
Labels
help wanted Extra attention is needed

Comments

@gavanderhoorn
Copy link
Member

The current implementation compares the requested timestamp to the author date of the commits in the ros/rosdistro log:

# TODO: do we want author or committer date here? Using author date for now.
ROSDISTRO_COMMIT_EPOCH=$(git -C "${ROSDISTRO_DIR}" log -n1 --date=format-local:'%s' --pretty=format:'%ad' ${ROSDISTRO_COMMIT})

So the ros/rosdistro revision the time-machine will make a cache for is the one which has a commit in it which was authored either at or immediately before the requested datetime.

It's unclear whether this could potentially be problematic: theoretically ros/rosdistro committers could push commits authored a long time ago. While one could argue that this possibility is slim (which is most likely true) and it doesn't matter (as the state of the machine of the author of the ros/rosdistro change (and consequently the ROS package versions and dependencies) would still match those in which we are interested), it may be more correct to use the committer date (as it would better encode the time and date at which the "state of ROS" would have changed because of the commits by the committer.

@gavanderhoorn
Copy link
Member Author

@wasowski @ChrisTimperley @git-afsantos @ipa-hsd: thoughts?

@gavanderhoorn
Copy link
Member Author

Based on our discussion in the robust meeting (2020-08-26), we'll go with committer date instead of the current author date.

Rationale: committer date would be when changes (local to someone's clone of rosdistro) end up in the main repository. At that point "the state of ROS" as a whole is changed. Not when the author of the change committed it to his local clone.

I'll put together a PR changing this and then @-mention all of you.

@gavanderhoorn gavanderhoorn added the help wanted Extra attention is needed label Aug 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

1 participant