Skip to content
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: update readme code example to new API. #382

Merged
merged 2 commits into from
Jan 6, 2023

Conversation

saul-jb
Copy link
Contributor

@saul-jb saul-jb commented Dec 11, 2022

This PR simply fixes the code example provided by the readme.

@saul-jb saul-jb requested a review from a team as a code owner December 11, 2022 19:14
@CLAassistant

This comment was marked as outdated.

Copy link
Member

@mpetrunic mpetrunic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign CLA agreement

README.md Show resolved Hide resolved
@saul-jb
Copy link
Contributor Author

saul-jb commented Dec 14, 2022

Please sign CLA agreement

I don't believe these CLAs are healthy to the opensource ecosystems since they are non-standard and contain legalese that only someone with good understanding of it can fully understand the extent of its meaning. Therefore I will not be signing it, however someone who has the ability to make the changes should update the readme to reflect the new API in a similar fashion to what I have done.

@wemeetagain
Copy link
Member

Not sure why we have the CLA here since this is a permissively licensed repo 🤔

@saul-jb saul-jb changed the title Update readme code example to new API. docs: Update readme code example to new API. Dec 15, 2022
@mpetrunic
Copy link
Member

Not sure why we have the CLA here since this is a permissively licensed repo 🤔

I think because of that ethereum c++ client that couldn't change license as external contributors were claiming that they only submitted code under previous license so CLA forces everyone to give out any rights to submitted code to chainsafe

@wemeetagain
Copy link
Member

Shouldn't that only be an issue when code isn't permissively licensed?

The c++ ethereum client was GPL licensed.

@codecov-commenter
Copy link

Codecov Report

Base: 83.29% // Head: 83.26% // Decreases project coverage by -0.03% ⚠️

Coverage data is based on head (91ef484) compared to base (23d49be).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #382      +/-   ##
==========================================
- Coverage   83.29%   83.26%   -0.04%     
==========================================
  Files          48       48              
  Lines       11779    11787       +8     
  Branches     1265     1266       +1     
==========================================
+ Hits         9811     9814       +3     
- Misses       1968     1973       +5     
Impacted Files Coverage Δ
src/metrics.ts 21.91% <0.00%> (-0.13%) ⬇️
src/tracer.ts 77.45% <0.00%> (-0.06%) ⬇️

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.
📢 Do you have feedback about the report comment? Let us know in this issue.

@philknows
Copy link
Member

Please sign CLA agreement

I don't believe these CLAs are healthy to the opensource ecosystems since they are non-standard and contain legalese that only someone with good understanding of it can fully understand the extent of its meaning. Therefore I will not be signing it, however someone who has the ability to make the changes should update the readme to reflect the new API in a similar fashion to what I have done.

We've agreed that CLA agreements for Lodestar should not exist on libraries such as this one, but only retained for our main Lodestar monorepo. @q9f is in the process of helping us to remove this requirement and hopefully that will allow this PR to be merged if there's no additional issues.

@q9f
Copy link

q9f commented Jan 5, 2023

You can ignore CLA here. It's not meant for the libraries.

@wemeetagain wemeetagain changed the title docs: Update readme code example to new API. docs: update readme code example to new API. Jan 6, 2023
@wemeetagain wemeetagain changed the title docs: update readme code example to new API. chore: update readme code example to new API. Jan 6, 2023
@wemeetagain wemeetagain merged commit b24d1ff into ChainSafe:master Jan 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants