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

Entry fixes #131

Merged
merged 3 commits into from
Jul 31, 2024
Merged

Entry fixes #131

merged 3 commits into from
Jul 31, 2024

Conversation

LordTermor
Copy link
Collaborator

This fixes some desc-file parsing logic so manually uploaded packages will get proper descriptions in database.

@LordTermor LordTermor added bug Something isn't working Daemon Changes related to the backend labels Jul 29, 2024
@LordTermor LordTermor requested a review from romangg July 29, 2024 17:59
@LordTermor LordTermor self-assigned this Jul 29, 2024
@romangg
Copy link
Contributor

romangg commented Jul 29, 2024

Is there an issue ticket for these fixes?

@LordTermor
Copy link
Collaborator Author

No.

It was an issue I've found and decided to fix right after that as it's pretty major

@romangg
Copy link
Contributor

romangg commented Jul 29, 2024

  • line length of commit messages must be 90 chars or less
  • can you describe the first commit better? I don't get the commit message and why it didn't work before.

@LordTermor
Copy link
Collaborator Author

line length of commit messages must be 90 chars or less

from error it sounds like a body line limit

body's lines must not be longer than 90 characters

That sounds like a very weird limitation. What would be the point? Should body be multilined manually then?

can you describe the first commit better? I don't get the commit message and why it didn't work before.

Package entities used to contain only file path and version but with 8976f71 I've added full desc contents to it because it's anyway stored like that and loading desc from archive on persistence step is costy. But I forgot to update some functions so they resulted incomplete package entity on manual upload. This PR fixes it.

@romangg
Copy link
Contributor

romangg commented Jul 30, 2024

That sounds like a very weird limitation. What would be the point?

Traditionally there are good reasons to have some limit. I use 90 instead of 72 because displays are larger nowadays and max code line lengths in projects have typically also increased from 80 to something between 100 and 120.

Should body be multilined manually then?

Yes, there is a setting though for vim for example to do it automatically.

Package entities used to contain only file path and version but with 8976f71 I've added full desc contents to it because it's anyway stored like that and loading desc from archive on persistence step is costy. But I forgot to update some functions so they resulted incomplete package entity on manual upload. This PR fixes it.

Thanks, that makes it clearer. For things like this having regression tests would be nice.

Pool entries used to have only versions but now include whole .desc-files. Mapper should
not be sufficed with version only anymore.
Signature path handling logic uses early return if signature was provided to
`parse_file_path` which caused .desc file to never load in that case.
Loading .desc from package files every time RecordMapper::to_entity called is unnecessary
and leads to massive performance issues. It's now being retrieved from the database.
@LordTermor LordTermor merged commit fe3cb3a into anydistro:master Jul 31, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Daemon Changes related to the backend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants