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
Bug 1748061: copy gotdotenv and add ability to override underlying scanner buffer … #177
Bug 1748061: copy gotdotenv and add ability to override underlying scanner buffer … #177
Conversation
1c0cb8b
to
36e2394
Compare
terraform / aws flake e2e-aws-builds /test e2e-aws-builds |
@gabemontero: This pull request references Bugzilla bug 1748061, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
per discussion in bugzilla removed WIP and associated with bug @adambkaplan bump / PTAL |
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.
@gabemontero is it possible to craft a patch to the godotenv repo?
From what I can gather this project is active, though it has fairly low velocity.
Presumably yes @adambkaplan as I noted in the description ... merely a question of expediency. Note, it would require a new "API", as the parameters for size would have to be passed down somehow ... so I could envision some time spent going back and forth nailing that down. Is it worth spending that time at this point? |
/lgtm /hold @gabemontero since this is for 4.4 with no plan to backport, I'd rather us be good citizens and propose a patch. If it takes too long we can merge this as is. |
36e2394
to
bac54ae
Compare
/hold cancel per discussion with @adambkaplan an upstream contribution would require changes to their public API (as public method signatures would have to change), or we would need klunky I've updated comments in code to explain the situation @adambkaplan bump / ptal |
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.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: adambkaplan, gabemontero The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest Please review the full test history for this PR and help us cut down flakes. |
2 similar comments
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
@gabemontero: All pull requests linked via external trackers have merged. Bugzilla bug 1748061 has been moved to the MODIFIED state. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
…for big files
/assign @adambkaplan
this is what would be required to fix https://bugzilla.redhat.com/show_bug.cgi?id=1748061
or alternatively, we submit an PR to github.com/joho/godotenv to add this capability and see if they accept it
Per my question in the bug, do we want to take this on given there is a customer case, or close as wontfix ... still waiting from feedback in bug on their feelings wrt to the workaround of using
--param
instead of--param-file