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

Integrate go-dag-jose plugin #8569

Merged
merged 8 commits into from
Nov 30, 2021
Merged

Integrate go-dag-jose plugin #8569

merged 8 commits into from
Nov 30, 2021

Conversation

smrz2001
Copy link
Contributor

@smrz2001 smrz2001 commented Nov 25, 2021

Integrate go-dag-jose plugin

Description

This PR integrates the go-dag-jose plugin into IPFS.

How Has This Been Tested?

  • ipfs dag put --store-codec dag-jose and ipfs dag get work correctly
  • All IPLD dag-jose fixtures work correctly

Definition of Done

  • My code follows conventions, is well commented, and easy to understand
  • My code builds and tests pass without any errors or warnings
  • I have tagged the relevant reviewers
  • The changes have been communicated to interested parties
  • Use the released version of go-dag-jose

References:

@welcome
Copy link

welcome bot commented Nov 25, 2021

Thank you for submitting this PR!
A maintainer will be here shortly to review it.
We are super grateful, but we are also overloaded! Help us by making sure that:

  • The context for this PR is clear, with relevant discussion, decisions
    and stakeholders linked/mentioned.

  • Your contribution itself is clear (code comments, self-review for the
    rest) and in its best form. Follow the code contribution
    guidelines

    if they apply.

Getting other community members to do a review would be great help too on complex PRs (you can ask in the chats/forums). If you are unsure about something, just leave us a comment.
Next steps:

  • A maintainer will triage and assign priority to this PR, commenting on
    any missing things and potentially assigning a reviewer for high
    priority items.

  • The PR gets reviews, discussed and approvals as needed.

  • The PR is merged by maintainers when it has been approved and comments addressed.

We currently aim to provide initial feedback/triaging within two business days. Please keep an eye on any labelling actions, as these will indicate priorities and status of your contribution.
We are very grateful for your contribution!

@warpfork
Copy link
Member

@aschmahmann suggests that we should also add a smoke tests that all the wiring works, by using a sharness test: https://github.com/ipfs/go-ipfs/blob/master/test/sharness/t0280-plugin-git.sh is an example for git. Can we quickly add something similar for this?

@smrz2001
Copy link
Contributor Author

@aschmahmann suggests that we should also add a smoke tests that all the wiring works, by using a sharness test: https://github.com/ipfs/go-ipfs/blob/master/test/sharness/t0280-plugin-git.sh is an example for git. Can we quickly add something similar for this?

Had to wrangle bash into submission so took a little longer than I expected. Please take a look @aschmahmann and @warpfork.

Also, I do see the failing CI tests so will work on fixing them ASAP.

@BigLep BigLep linked an issue Nov 30, 2021 that may be closed by this pull request
3 tasks
@BigLep
Copy link
Contributor

BigLep commented Nov 30, 2021

@smrz2001: @aschmahmann is going to rebase first since it's quite possible the CI failures aren't your fault. It looks like you may have forked at an inopportune time while we were getting interoperability tests to pass.

@aschmahmann
Copy link
Contributor

@smrz2001 I don't have permissions to push to your branch can you give me permissions (https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork) or do the rebase yourself?

@smrz2001
Copy link
Contributor Author

Thanks, @BigLep.

Hey @aschmahmann, just looking at this. I can't find that checkbox for some reason 😕 Give me a couple min to figure this out, otherwise I'll try the rebase myself.

@smrz2001
Copy link
Contributor Author

omfg, I hate WSL (Windows Subsystem for Linux) sometimes. One of the submodules keeps messing up my attempts to do anything because it shows up as a diff when there is none.

I just added you to my fork, @aschmahmann.

warpfork and others added 8 commits November 30, 2021 13:26
This commit is not functional on its own.
Some features are missing upstream.
Someone will need to take this as a guide and do additional work
to make it complete and functional,
once the needed upstream features are available.
@aschmahmann aschmahmann merged commit ecd2628 into ipfs:master Nov 30, 2021
@smrz2001 smrz2001 deleted the feat/dagjose branch November 30, 2021 19:00
@stbrody
Copy link

stbrody commented Nov 30, 2021

🥳 🎉

Thanks @aschmahmann for the quick turnaround!

@aschmahmann aschmahmann mentioned this pull request Dec 1, 2021
80 tasks
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.

Support dag-jose codec by default
5 participants