PHP Composer.json Extractor#60
Conversation
| return best, found | ||
| } | ||
|
|
||
| func GetMinimumVersion(constraint string) (string, error) { |
There was a problem hiding this comment.
I would add a lot of unit tests for this, also (as per linter) add some docs/examples
There was a problem hiding this comment.
that will be covered in the PR for deps.dev/utils/resolve. this implementation is just a placeholder until that gets merged
alessandro-Doyensec
left a comment
alessandro-Doyensec
left a comment
There was a problem hiding this comment.
Hello @v1ktor0t
I added a lot of suggestions on the style, the logic seems fine.
I would guess that a lot of the code would change to interact better with the depsutil library so don't stress to much on resolving the conversations here.
|
One last suggestion: try to avoid using alias types (ex: I usually stick with using: type versions struct{
value [4]int
} |
|
@alessandro-Doyensec, I implemented most of the recommended updates. I'd appreciate a second look when you get the chance. |
|
|
||
| func (v version) String() string { | ||
| parts := []string{strconv.Itoa(v.parts[0]), strconv.Itoa(v.parts[1]), strconv.Itoa(v.parts[2])} | ||
| if v.parts[3] != 0 { |
There was a problem hiding this comment.
It might make sense to include the length of the version inside of the version struct since there might be a difference between:
v3.1.0 and v3.1
(Not sure though)
There was a problem hiding this comment.
(having the length of the version inside the struct also simplifies the parseDigits logic)
| func (v version) String() string { | ||
| parts := []string{strconv.Itoa(v.parts[0]), strconv.Itoa(v.parts[1]), strconv.Itoa(v.parts[2])} | ||
| if v.parts[3] != 0 { | ||
| parts = append(parts, strconv.Itoa(v.parts[3])) | ||
| } | ||
| return strings.Join(parts, ".") | ||
| } |
There was a problem hiding this comment.
The String func can be rewritten using fmt.Sprint()
| comparators := []comparator{{">=", fromVersion}} | ||
| // upper bound is fully specified. inclusive comparison | ||
| if toVersionN >= 3 { | ||
| comparators = append(comparators, comparator{"<=", toVersion}) |
There was a problem hiding this comment.
Early return:
return append(comparators, comparator{"<=", toVersion})|
|
||
| // comparator is one primitive comparison; a constraint parses to an OR-list of AND-groups of these. | ||
| type comparator struct { | ||
| op string |
There was a problem hiding this comment.
could you rename this operand, maybe it's my fault but it wasn't clear when i first read it
|
Overall looks good, I just added a couple of nit/style comments on the version file. Beside that ready to be shipped 🚀 |
2 participants
Extractor for PHP's Composer. I'd appreciate a set of eyes on the initial version.
Note: I'm working on a PR for the version resolution code to be merged in the deps.dev/utils/resolve package.