This repository was archived by the owner on May 2, 2024. It is now read-only.
Open
Conversation
Codecov Report
@@ Coverage Diff @@
## oxidation #61 +/- ##
==============================================
- Coverage 100.00% 57.01% -42.99%
==============================================
Files 1 7 +6
Lines 22 328 +306
==============================================
+ Hits 22 187 +165
- Misses 0 141 +141
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
saenai255
force-pushed
the
feat/persistance-system
branch
from
b3086fc to
9a6a054
Compare
wizard-28
suggested changes
Sep 5, 2022
Member
wizard-28
left a comment
wizard-28
left a comment
There was a problem hiding this comment.
Well, that's my quick review, this PR will take a few more days to fully review.
General impressions:
- Add more docs, the whole
fsmodule's missing docs. - Look into error handling libraries
- 13 of the 16 tests fail, check the
Code CoverageCI.
saenai255
force-pushed
the
feat/persistance-system
branch
from
4374d8d to
1074520
Compare
saenai255
force-pushed
the
feat/persistance-system
branch
from
ecdd8e3 to
7668b2a
Compare
wizard-28
suggested changes
Oct 4, 2022
Co-authored-by: Sourajyoti Basak <wiz28@protonmail.com>
Co-authored-by: Sourajyoti Basak <wiz28@protonmail.com>
Co-authored-by: Sourajyoti Basak <wiz28@protonmail.com>
Co-authored-by: Sourajyoti Basak <wiz28@protonmail.com>
Co-authored-by: Sourajyoti Basak <wiz28@protonmail.com>
Co-authored-by: Sourajyoti Basak <wiz28@protonmail.com>
Co-authored-by: Sourajyoti Basak <wiz28@protonmail.com>
Co-authored-by: Sourajyoti Basak <wiz28@protonmail.com>
…tall into feat/persistance-system
wizard-28
suggested changes
Oct 10, 2022
Member
wizard-28
left a comment
wizard-28
left a comment
There was a problem hiding this comment.
Add code examples to the public methods detailing how to use them.
Also, add a full-fledged example(s) on how to use the database module, in examples/ folder on the root of the project.
So people can run cargo run --example <database_example> to run the example.
Co-authored-by: Sourajyoti Basak <wiz28@protonmail.com>
wizard-28
suggested changes
Oct 29, 2022
Member
wizard-28
left a comment
wizard-28
left a comment
There was a problem hiding this comment.
Can you rename the PacBuild fields to resemble their PACBUILD counterparts wherever possible? Example: package_names -> pkgname, homepage -> url, licenses -> license
Comment on lines
+43
to
+47
| /// Repology filter. | ||
| /// | ||
| /// # Example | ||
| /// **TBA** | ||
| pub repology: String, |
Member
There was a problem hiding this comment.
Shouldn't this be a HashMap of the filters?
| /// | ||
| /// # Example | ||
| /// `Paul Cosma <paul.cosma97@gmail.com>` | ||
| pub maintainers: Vec<String>, |
Member
There was a problem hiding this comment.
Suggested change
| pub maintainers: Vec<String>, | |
| pub maintainers: Vec<String>, |
Shoud be optional
| pub description: String, | ||
|
|
||
| /// Official homepage [URL]. | ||
| pub homepage: URL, |
| pub homepage: URL, | ||
|
|
||
| /// Latest version fetched from Repology. | ||
| pub repology_version: Version, |
Member
There was a problem hiding this comment.
Why should we store the repology version?
Comment on lines
+57
to
+59
| /// An array of packages that must be installed for the software to build | ||
| /// and run. | ||
| pub dependencies: Vec<VersionConstrainedPackageId>, |
Comment on lines
+86
to
+88
| /// The group the package belongs in. For instance, when installing | ||
| /// `plasma`, it installs all packages belonging in that group. | ||
| pub groups: Vec<GroupId>, |
Comment on lines
+90
to
+92
| /// Optional dependencies. Each Key:Pair is meant to describe the package | ||
| /// identifier and the reason for installing. | ||
| pub optional_dependencies: HashMap<VersionConstrainedPackageId, String>, |
Comment on lines
+94
to
+95
| /// An array of packages that are only required to build the software. | ||
| pub make_dependencies: Vec<VersionConstrainedPackageId>, |
Comment on lines
+97
to
+98
| /// The license under which the software is distributed. | ||
| pub licenses: Vec<String>, |
Comment on lines
+100
to
+101
| /// File required to build the package. | ||
| pub url: URL, |
Member
There was a problem hiding this comment.
Do we need to store the url in the database?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #28