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

Add sponsor command #1258

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

xTrayambak
Copy link
Contributor

@xTrayambak xTrayambak commented Aug 14, 2024

Prelude

See this issue
This PR exists as a base for what can be the final implementation for this. Do NOT merge this yet!
This PR adds a new command: nimble sponsor

The sponsor command

The sponsor command optionally takes in a package as an argument. If one is not provided, it shows all packages that are accepting donations (the package below does not actually take donations, it's just been modified to show how this command works in my packages_official.json)
image

If a package is provided, the links for sending donations to the library maintainer are opened in the user's browser and also outputted to their terminal if they don't have a web browser installed. (Again, the package below does not actually take donations.)
image

case meth
of DonationMethod.GitHub: "GitHub"
of DonationMethod.OpenCollective: "OpenCollective"
of DonationMethod.Patreon: "Patreon"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be just a string metadata instead of just 3 hardcoded "provider companies",
some people prefer to use USDT/BTC/ETH/crypto/other services etc,
and in some places of the world the hardcoded services are not available (as donations collector).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds alright, but we'd also need to properly audit any new packages that are added in. We need to ensure that none of them end up linking to a phishing site, albeit the odds are very low.

Copy link
Contributor Author

@xTrayambak xTrayambak Aug 14, 2024

Choose a reason for hiding this comment

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

image
image

@juancarlospaco Is this better? (I have no clue about how Coinbase works, that's just a stub link)

Copy link
Contributor

Choose a reason for hiding this comment

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

Thats what I tried to do at
https://github.com/juancarlospaco/nim_packages_security_audit?tab=readme-ov-file#fully-automated-nim-packages-security-audit
Feel free to contribute/revive, maybe it needs like a "cool web visualization" (?).

Also the package scanner can be improved too:
https://github.com/nim-lang/packages/blob/master/package_scanner.nim

I think the efforts there are so much better spent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This might sound stupid - but what about using something like URL Haus, VirusTotal or Google Safebrowsing to the package scanner?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nevermind, URL Haus does not keep track of phishing URLs (my main concern).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@juancarlospaco I'd say we should leave the URL auditing to the person who's going to merge the package into the index. Most browsers have SafeBrowsing built in either ways. Do you have any other suggestions?

Copy link
Contributor

Choose a reason for hiding this comment

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

Enforce a common-sense standard max len for the string ? 256 char or something here or in the package scanner. IMHO 🤔

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.

2 participants