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

Add a newline between data json objects to prevent flagging as unchanged. #465

Merged
merged 1 commit into from
Jan 23, 2025

Conversation

rkent
Copy link

@rkent rkent commented Jan 23, 2025

I believe this fixes #463 but I have not had time to test that (which is quite tricky). But here is an explanation.

In the current code, all of the data and index .json lines are written as a single line. So if anything changes, git diff always reports this as just one line changed.

In git_add_files, changes are excluded from update if they only change a single line, and if the diff contains "generated on".

In humble, the README for package proto2ros contains the text "a ROS 2 message is generated for each one-of". Frequent word deletion removes "for each" so this becomes "generated one-of" which contains "generated on". Thus data.humble.json is blacklisted from update. So this problem is a side effect of including the README text from #452.

The proposed patch adds new-lines to each new object, so that data json files with changes will not be detected as single line changes (or so I believe). There is a small additional size added to the data json files with this.

It may take a day or two for me to test this as much as I can, but it is a pretty small change so might be worth landing as it.

@rkent
Copy link
Author

rkent commented Jan 23, 2025

I tested this now, and AFAICT it works.

Copy link
Member

@tfoote tfoote left a comment

Choose a reason for hiding this comment

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

Thanks for this change. Making the file multi-line will potentially also help with storage in git as well where index updates can also be reduced to diffs in the output repo instead of every diff having the full contents of the index due to the full line change.

Thanks for verifying it locally. I'll merge this and force a rebuild to check it this afternoon.

@tfoote tfoote merged commit 07e9e5b into ros-infrastructure:ros2 Jan 23, 2025
1 check passed
@rkent rkent deleted the newline-in-data-json branch January 23, 2025 22:09
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.

Search Index missing items
2 participants