Skip to content

revision: remove unused Revision.commit_id field (Bug 1995895)#632

Closed
cgsheeh wants to merge 1 commit intomozilla-conduit:mainfrom
cgsheeh:remove-commit-id
Closed

revision: remove unused Revision.commit_id field (Bug 1995895)#632
cgsheeh wants to merge 1 commit intomozilla-conduit:mainfrom
cgsheeh:remove-commit-id

Conversation

@cgsheeh
Copy link
Copy Markdown
Member

@cgsheeh cgsheeh commented Oct 22, 2025

No description provided.

# A general purpose data field to store arbitrary information about this revision.
data = models.JSONField(blank=True, default=dict)

# The commit ID generated by the landing worker, before pushing to remote repo.
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

After taking a quick look at adding commit_id to the intermediate table, I noticed that we set the diff_id on the intermediate table by using the currently set Revision.diff_id. I think if we wanted to use the same pattern for setting the commit_id, we would need to keep this field around and update it. Happy to set the revision IDs using an argument to the method instead, just wanted to point that out. 👍

Copy link
Copy Markdown
Contributor

@zzzeid zzzeid Oct 23, 2025

Choose a reason for hiding this comment

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

Hm yeah I think it makes sense to do it the same way (maybe even in the same method, after renaming the method.)

EDIT: that, or re-implement both in a different way, just think it would make sense to be consistent.

@cgsheeh cgsheeh requested a review from zzzeid October 22, 2025 22:54
@cgsheeh cgsheeh closed this Oct 23, 2025
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.

2 participants