Skip to content

Conversation

@viktordick
Copy link
Collaborator

@Wiseqube Do you remember why you included this in the excluded keys? We now have a lot of systems yielding unstaged changes because of this. Was there a reason or was this an accident?

@Wiseqube
Copy link
Contributor

Wiseqube commented Nov 1, 2025

@viktordick first I've heard of it (the unstaged changes, that is) I think the reason was, du to the fact that we also included a 'writeback' to the Data.FS on record I was getting this weird 'is_root' property on the git repos __root__/__meta__ file which was not there before, so since this was only a zodbsync internal key on the root object I excluded it.

@viktordick
Copy link
Collaborator Author

@Wiseqube But this is_root property was part of recordings in the past, why change this behavior?

@viktordick
Copy link
Collaborator Author

Not sure I understand your response. The is_root was included since the very beginning (the very first commit in 2015 of dbutils-zoperepo) and has been there since then, only changin the position at some point. With this change, it is no longer there - resulting in a superfluous change that yields unstaged changes and Generic Commit mails.

I guess you are saying that due to the writeback we now also have an is_root in the props list? I'll have to check that.

@Wiseqube
Copy link
Contributor

Wiseqube commented Nov 1, 2025

Huh, ok then really not sure why... I thought that was the reason but I guess I'm wrong.

PS: No, I meant the key in the __meta__ file dictionary, so you are right saying that it has been there all along. I can't piece together a reason why this is there. Just confused why I have not seen the issues described yet somewhere.

@viktordick
Copy link
Collaborator Author

Checked it and there is no adverse effect of this PR, it simply restores the previous behaviour of adding an is_root field to the top-level element in the recording, avoiding unnecessary changes.

If you agree, could you approve? I'd create a new release and we can roll it out before too many systems are faced with this.

@Wiseqube Wiseqube self-requested a review November 1, 2025 11:48
Copy link
Contributor

@Wiseqube Wiseqube left a comment

Choose a reason for hiding this comment

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

LGTM

@viktordick viktordick merged commit 6eff0b5 into main Nov 1, 2025
6 checks passed
@viktordick viktordick deleted the re-include-is-root branch November 1, 2025 12:02
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.

3 participants