Improve documentation to prepare for 0.3 release #315
Conversation
src/report.rs
Outdated
| /// and refer to the explanation for the other times. | ||
| /// Indicate if the incompatibility is present multiple times in the derivation tree. | ||
| /// | ||
| /// If that is the case, the number is a unique id. We may want to only explain this |
There was a problem hiding this comment.
Are we consistent in our documentation about whether "we" means the maintainers of pubgrub or the users?
src/version_set.rs
Outdated
| /// Two version sets that contain the same versions must be equal. | ||
| /// | ||
| /// The methods with default implementations can be overwritten for better performance, but they | ||
| /// must be equal to the default implementation. |
There was a problem hiding this comment.
It took me two readings to figure out what was meant by the sentence.
Perhaps "equivalent" or "functionally equivalent" instead of "equal".
There was a problem hiding this comment.
As I remember, equality was needed because version set comparisons serves as building blocks to check if sets are subsets of others in parts of the pubgrub algorithm. And as I remember it too, we have some text describing this already in some our default implementation of this?
There was a problem hiding this comment.
I've done another pass over the version set contract
|
These look like good improvements to me. |
konstin
force-pushed
the
konsti/dev/prepare-0.3
branch
from
c520325 to
e19f767
Compare
|
I am +1 on this; @mpizenberg can merge when he is happy with it. |
| /// stored in a canonical representations. Such problems may arise if your implementations of | ||
| /// `complement()` and `intersection()` do not return canonical representations. | ||
| /// | ||
| /// For example, `>=1,<4 || >=2,<5` and `>=1,<4 || >=3,<5` are equal, because they can both be |
There was a problem hiding this comment.
Perhaps explicitly call out that this example is for integers.
There was a problem hiding this comment.
I thought about them in the context of PEP 440 and semver, are there cases when this doesn't hold?
| @@ -71,6 +79,7 @@ pub trait VersionSet: Debug + Display + Clone + Eq { | |||
| } | |||
There was a problem hiding this comment.
Small grammar things on these two.
Whether these ranges have no overlapping segments.
and
Whether all elements contained in
selfare contained inother.
|
I have read through all the docs for this library. Except for the few things I commented on, they're in good shape for a 0.3. Next to read the guide. |
|
The other fixes can be done in a subsequent PR. I’m sure there will be multiple docs PRs anyway once 0.3 is released and people start reading through it more thoroughly |
While updating the guide, i went through the referenced code and updated their docs.
An outstanding issue are the associated types on
DependencyProvider, their naming is consistent (PvsPriority,Mhas no long form,Erris in the middle).Part of #308