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

ndb: add verify method and cleanup code #1004

Merged
merged 7 commits into from Jan 10, 2020

Conversation

mlschroe
Copy link
Contributor

@mlschroe mlschroe commented Jan 9, 2020

This commit adds a verify method for ndb's Packages.db database.
It also cleans up the pkgdb code a bit:

  • removed unused lzo compression code
  • added some more comments and fixed spelling mistakes
  • made "ordered slots" flag a boolean
  • fixed a corner case where a package id lookup could segfault
    if no packages were in the database
  • no longer free the pkgid hash all the time

@pmatilai
Copy link
Member

Please split to individual commits.

@mlschroe
Copy link
Contributor Author

I kind of fail to see the point, but I'll do it anyways just to please you.

@pmatilai
Copy link
Member

pmatilai commented Jan 10, 2020

These are essentially unrelated changes, hence they belong to separate commits. It might not seem that valuable to the author, but it makes reviewing much easier, and the biggest value of strict commit-per-logical-change comes over time from bisecting, cherry-picking etc.

Lumping such changes into one pull-request is absolutely fine of course.

@mlschroe
Copy link
Contributor Author

I get that, but I was under the impression that ndb is still marked as experimental. (That's about to change in the near future, though.)

Anyway, force pushed to multiple commits.

We never needed to order by pkgid or slot number, so simplify
the code.
This is not used as we re-read all slots everytime a package
is added or deleted.
Reuse the hash if the size matches. This was actually the original
intention, but for some reason the code was disabled by always
freeing the hash.
This adds a verify method for ndb's Packages.db database. The
Index.db database is currently not verified.
This seems to saner as the somewhat unreliable current time.
This element is not used in normal database operation, it is
for repair tools that need to decide which blob to take if two
blobs have the same pkgid.
@pmatilai
Copy link
Member

Oh, I don't see experimental status affecting change-per-commit requirement at all, as the commits will remain forever regardless of the status. Various other aspects can be somewhat relaxed on experimental code of course.

Anyway, splitting up much appreciated, thanks.

@pmatilai pmatilai merged commit f674af7 into rpm-software-management:master Jan 10, 2020
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.

None yet

2 participants