[chore]: dont set path for go in validate #6273
Conversation
Signed-off-by: sakshi1215 <[email protected]>
Signed-off-by: sakshi1215 <[email protected]>
|
@dmitryax can you please trigger the CI on this? Also, I am unable to sign CLA as individual contributor. |
|
I could trigger this, but I don't think this is the right solution -- the mismatch is that the name "Validate" implies a function that does not change the state. Setting the path is indeed correct, so perhaps we just need to change the name of the function? |
|
Oh I might have misunderstood the context of the same, let me make the
changes you suggested.
…On Tue, 11 Oct 2022 at 7:35 PM, Juraci Paixão Kröhling < ***@***.***> wrote:
I could trigger this, but I don't think this is the right solution -- the
mismatch is that the name "Validate" implies a function that does not
change the state. Setting the path is indeed correct, so perhaps we just
need to change the name of the function?
—
Reply to this email directly, view it on GitHub
<#6273 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ANF3CEFJ4RK7I3CXTRRACNLWCVX2XANCNFSM6AAAAAARB562HU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Signed-off-by: sakshi1215 <[email protected]>
|
@jpkrohling Can you please check again? |
Codecov ReportBase: 92.30% // Head: 91.79% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #6273 +/- ##
==========================================
- Coverage 92.30% 91.79% -0.52%
==========================================
Files 219 235 +16
Lines 13463 13454 -9
==========================================
- Hits 12427 12350 -77
- Misses 806 876 +70
+ Partials 230 228 -2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Signed-off-by: sakshi1215 <[email protected]>
sakshi1215
changed the title
Co-authored-by: Dmitrii Anoshin <[email protected]>
|
Can we merge this @dmitryax ? |
|
@jpkrohling are you ok with this? |
| // Validate checks whether the current configuration is valid | ||
| func (c *Config) Validate() error { | ||
| // ValidateAndSetGoPath checks whether the current configuration is valid and sets go path | ||
| func (c *Config) ValidateAndSetGoPath() error { |
There was a problem hiding this comment.
I think I would prefer to split this function into two:
- SetGoPath
- Validate
The current callers of Validate will then need to call both.
There was a problem hiding this comment.
Done @jpkrohling
Signed-off-by: Sakshi Patle <[email protected]>
|
@jpkrohling I have fixed it please check |
|
Please fix tests. |
Signed-off-by: Sakshi Patle <[email protected]>
sakshi1215
requested review from
dmitryax and
bogdandrutu
and removed request for
jpkrohling and
dmitryax
|
@bogdandrutu Please review this again, I have fixed tests |
Signed-off-by: sakshi1215 [email protected]
Description:
Removing setting path in validate
Link to tracking Issue: fixes #6030