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.

Sign up for GitHub

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

Jump to bottom

PR for Command: getDDO #113

Merged
merged 6 commits into from
Nov 16, 2023
Merged

PR for Command: getDDO #113

merged 6 commits into from
Nov 16, 2023

Conversation

an7e
Copy link
Contributor

@an7e an7e commented Nov 15, 2023 •
edited
Loading

Issue #49

Changes proposed in this PR:

  • added command getDDO
  • added ddo handler
  • added this context to work in methods

@an7e an7e linked an issue Nov 15, 2023 that may be closed by this pull request
Command: getDDO #49
Closed
3 tasks
@an7e an7e marked this pull request as ready for review November 15, 2023 16:03
mariacarmina
mariacarmina reviewed Nov 16, 2023
View reviewed changes
Copy link
Member

@mariacarmina mariacarmina 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 from my side. One question though:
Does this PR also depend on #116 for finding the DDO first and then using this PR to serve it? Should we wait for #116 or it can be merged separately?
One mention is that please merge the develop branch in command-getddo.

@alexcos20
Copy link
Member

can you rename the PR please? :)

@an7e an7e changed the title new command PR for Command: getDDO Nov 16, 2023
alexcos20
alexcos20 approved these changes Nov 16, 2023
View reviewed changes
Copy link
Member

@alexcos20 alexcos20 left a comment

Choose a reason for hiding this comment

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

lgtm, although it would be nice to have some tests

@an7e
Merge remote-tracking branch 'origin/develop' into command-getddo
1809dff 
# Conflicts:
#	src/components/P2P/handleProtocolCommands.ts
#	src/components/httpRoutes/validateCommands.ts
#	src/utils/constants.ts
paulo-ocean
paulo-ocean approved these changes Nov 16, 2023
View reviewed changes
Copy link
Contributor

@paulo-ocean paulo-ocean left a comment

Choose a reason for hiding this comment

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

green light! 👍

@an7e an7e merged commit d6bbc1a into develop Nov 16, 2023
5 checks passed
@an7e an7e deleted the command-getddo branch November 16, 2023 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mariacarmina mariacarmina mariacarmina left review comments

@paulo-ocean paulo-ocean paulo-ocean approved these changes

@alexcos20 alexcos20 alexcos20 approved these changes

@bogdanfazakas bogdanfazakas Awaiting requested review from bogdanfazakas bogdanfazakas is a code owner

@jamiehewitt15 jamiehewitt15 Awaiting requested review from jamiehewitt15 jamiehewitt15 is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Command: getDDO
4 participants
@an7e @alexcos20 @mariacarmina @paulo-ocean