-
Notifications
You must be signed in to change notification settings - Fork 954
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
chore(Makefile): detect and default to make go-install on darwin vs install everywhere else #3197
Conversation
…ll other cases for refs #3169
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3197 +/- ##
==========================================
- Coverage 51.81% 51.49% -0.32%
==========================================
Files 178 178
Lines 11286 11286
==========================================
- Hits 5848 5812 -36
- Misses 4930 4968 +38
+ Partials 508 506 -2 ☔ View full report in Codecov by Sentry. |
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. The suggestion I made is regarding Makefile comments
for docs, should i just use default for ubuntu/mac as |
Co-authored-by: Rootul P <rootulp@gmail.com>
…nstall everywhere else (celestiaorg#3197) <!-- Thank you for submitting a pull request! Please make sure you have reviewed our contributors guide before submitting your first PR. Please ensure you've addressed or included references to any related issues. Tips: - Use keywords like "closes" or "fixes" followed by an issue number to automatically close related issues when the PR is merged (e.g., "closes #123" or "fixes #123"). - Describe the changes made in the PR. - Ensure the PR has one of the required tags (kind:fix, kind:misc, kind:break!, kind:refactor, kind:feat, kind:deps, kind:docs, kind:ci, kind:chore, kind:testing) --> refs celestiaorg#3169 Suggestion we add this little detect and switch. One issue i see with attempting to detect GOBIN is that it doesn't HAVE to be set (ie: there can be a default). This i would think is a good quick compromise to un-confuse @rootulp and (potentially) other devs and make this behavior more delightful most of the time. IF not, we should close this and the other issue. --------- Co-authored-by: Rootul P <rootulp@gmail.com>
…nstall everywhere else (celestiaorg#3197) <!-- Thank you for submitting a pull request! Please make sure you have reviewed our contributors guide before submitting your first PR. Please ensure you've addressed or included references to any related issues. Tips: - Use keywords like "closes" or "fixes" followed by an issue number to automatically close related issues when the PR is merged (e.g., "closes #123" or "fixes #123"). - Describe the changes made in the PR. - Ensure the PR has one of the required tags (kind:fix, kind:misc, kind:break!, kind:refactor, kind:feat, kind:deps, kind:docs, kind:ci, kind:chore, kind:testing) --> refs celestiaorg#3169 Suggestion we add this little detect and switch. One issue i see with attempting to detect GOBIN is that it doesn't HAVE to be set (ie: there can be a default). This i would think is a good quick compromise to un-confuse @rootulp and (potentially) other devs and make this behavior more delightful most of the time. IF not, we should close this and the other issue. --------- Co-authored-by: Rootul P <rootulp@gmail.com>
refs #3169
Suggestion we add this little detect and switch. One issue i see with attempting to detect GOBIN is that it doesn't HAVE to be set (ie: there can be a default). This i would think is a good quick compromise to un-confuse @rootulp and (potentially) other devs and make this behavior more delightful most of the time. IF not, we should close this and the other issue.