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

[PR] Version 0.01 #2

Merged
merged 22 commits into from
Nov 29, 2022
Merged

[PR] Version 0.01 #2

merged 22 commits into from
Nov 29, 2022

Conversation

LuchoTurtle
Copy link
Member

Addresses #1 but doesn't close it.

This is the initial version of the package. It offers encoding and decoding following the Multihash standards and it is fully tested (100%).

The README follows both Dart's guidelines and the structure of most of the Multihash repos, hence why it differs a wee from dwyl's.

Adding CI and badges ought to be added after merging this.

@LuchoTurtle LuchoTurtle added documentation Improvements or additions to documentation enhancement New feature or enhancement of existing functionality awaiting-review An issue or pull request that needs to be reviewed labels Nov 25, 2022
@LuchoTurtle LuchoTurtle self-assigned this Nov 25, 2022
@LuchoTurtle
Copy link
Member Author

@nelsonic I don't know if you want to review this or not (probably the latter) but I'll wait for a tweet from you. This will be used for the dart_cid and I want to get it published so I can use it as a dependency whilst developing it.

I can publish the package myself but to transfer it to a dwyl publisher, we need to create it. It involves using dwyls.com domain and Google Console, so if you want me to do that, just say so.

On to https://github.com/dwyl/dart_cid 👍

README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
dart_cid.iml Outdated Show resolved Hide resolved
Copy link
Member

@nelsonic nelsonic left a comment

Choose a reason for hiding this comment

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

@LuchoTurtle the code for this package is looking good so far. 👌
But it's not awaiting-review ... ⏳
Where are the GitHub Actions for running Continuous Integration (CI) Tests? 🤷‍♂️
Please don't assign a PR to me unless you are certain you've done everything you can to prepare it for my review. Thanks. 🙏

@nelsonic nelsonic assigned LuchoTurtle and unassigned nelsonic Nov 29, 2022
@nelsonic nelsonic added help wanted If you can help make progress with this issue, please comment! in-progress An issue or pull request that is being worked on by the assigned person and removed awaiting-review An issue or pull request that needs to be reviewed labels Nov 29, 2022
@LuchoTurtle
Copy link
Member Author

@nelsonic fixed the README and added CI according to your feedback. The CI appears to not run until it is merged to main so it can be fixed if anything goes awol after merging this PR.

@LuchoTurtle LuchoTurtle requested a review from nelsonic November 29, 2022 10:41
@LuchoTurtle LuchoTurtle assigned nelsonic and unassigned LuchoTurtle Nov 29, 2022
@LuchoTurtle LuchoTurtle added awaiting-review An issue or pull request that needs to be reviewed and removed in-progress An issue or pull request that is being worked on by the assigned person labels Nov 29, 2022
Copy link
Member

@nelsonic nelsonic left a comment

Choose a reason for hiding this comment

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

Looks good @LuchoTurtle 👌
Thanks for making the updates. 🧑‍💻
Let's try and ship this. :shipit: 🤞

@nelsonic nelsonic merged commit efc62ea into main Nov 29, 2022
@nelsonic nelsonic deleted the version-1 branch November 29, 2022 23:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review An issue or pull request that needs to be reviewed documentation Improvements or additions to documentation enhancement New feature or enhancement of existing functionality help wanted If you can help make progress with this issue, please comment!
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants