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 apt credentials parser #60
Add apt credentials parser #60
Changes from all commits
f573944
88c1fb0
cfcc048
ff67466
023a9a1
aea488e
a803cf3
15afa40
8be5a54
518af57
8c109e3
959c8c2
c6db9e7
552b996
dc4f7d9
715a51c
5f4627f
b50901e
010e969
5e335b1
482ff15
db609e1
ddefc45
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.
While we're still here, thanks for the extensive documentation and references above.
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.
Feels like this error needs checking, as it'd be surprising and error prone for a state to report a problem and it just be ignored.
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.
When
err != nil
thenstate == nil
, the loop breaks and the error is later returned, unless an error happened in scanner, in which case the parser error is consequence of it, and so the scanner error is returned instead. It would be a bug for a state to return bothstate
anderr
non-nil. Does it make sense?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.
that logic is true @woky.
Some questions though:
err != nil
andstate == nil
, then do we need to continue to line 221? If not, better check theerr
here and justreturn
It would be a bug for a state to return both state and err non-nil.
. So isn't that the check you should be doing? I.e.if state != nil && err != nil
?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.
We need to go to 221 because states return error when
scanner.Scan()
returns false which can happens on EOF or on an error condition. States treat the false return as EOF. We need to check if it's not an IO error.Those are two things. The
state != nil
is the algorithm. Thestate != nil && err != nil
is the algorithm AND assertion that checks for bugs. I've added the assertion.