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.
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
Add objects.inv #522
Add objects.inv #522
Changes from 31 commits
3dbbb7b
217c706
2cddffc
7c204c2
fbcbc93
4d6bd50
f609020
a35e55a
eb6d46d
87f15e1
7d90e7e
da3a2f8
490cd4b
aac8ca8
8ccd12d
ccc5456
0e88fbf
f1ad63d
f3da368
8dbb25a
a646ffe
ee58482
b953189
fda5840
869ec07
994f9ed
3014893
b189efd
ffef5fb
c28c20a
1c8fc2c
10f2c15
ada7bd4
6c66a6e
acee94e
79de476
5f79584
0131c9d
5078141
0698d16
12ceb68
0988f2c
81b0c17
8ab0bab
860e191
21b7582
ac85d66
6f6bcd4
b47fd42
08596b6
7315df4
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: you could assume that the file is always called
objects.inv
, so doObjectsInv.fromFile(htmlPath)
where the arg is calledparentDirectoryPath
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, incorporated in acee94e
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This logic appears in a few different places, might be nice to consolidate in a future refactor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe make it a static function:
ObjectsInv.determineDest(pkg: Pkg)
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could also have
.write()
call this andmkdirp
. Have.write()
take the argpkg: Pkg
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't want
ObjectsInv
to need to know aboutPkg
, partially for code decoupling but also to make it easier to write the temp file while testing.I think #630 is a good compromise. I'll pull in those changes if it's merged. Then this can just be
incorporated the acee94e, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See 81b0c17