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

feat: DeactivateDid transaction to mark DIDs as deactivated [DEV-1774] #332

Merged
merged 25 commits into from
Oct 26, 2022
Merged

feat: DeactivateDid transaction to mark DIDs as deactivated [DEV-1774] #332

merged 25 commits into from
Oct 26, 2022

Conversation

khornonzon
Copy link
Contributor

No description provided.

@khornonzon khornonzon changed the title tx.proto commit tx.proto commit (did-deactivate) Jun 17, 2022
@Toktar Toktar changed the title tx.proto commit (did-deactivate) Add DeactivateDid transaction Jun 17, 2022
@Toktar Toktar marked this pull request as draft June 17, 2022 19:30
Toktar
Toktar suggested changes Jun 17, 2022
View reviewed changes
Copy link
Contributor

@Toktar Toktar left a comment

Choose a reason for hiding this comment

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

Could you please start with a Plan of Attack?

Toktar
Toktar reviewed Jun 28, 2022
View reviewed changes
Toktar
Toktar reviewed Jun 28, 2022
View reviewed changes
Toktar
Toktar suggested changes Jul 5, 2022
View reviewed changes
Copy link
Contributor

@Toktar Toktar left a comment

Choose a reason for hiding this comment

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

Thank you very much for your contribution to our project, it is really valuable for us!
I have completed the code review. For merging the PR some changes should be done in accordance with the comments above and added system tests to the /tests/e2e-bash/tests/ folder to test the cli command

@ankurdotb ankurdotb changed the title Add DeactivateDid transaction feat!: DeactivateDid transaction to mark DIDs as deactivated Jul 22, 2022
@Tweeddalex
Copy link
Contributor

Hey @Toktar it would be good to understand what the status is here. Is the deactivate DID feature complete now? What are the next steps?

@Toktar Toktar marked this pull request as ready for review September 28, 2022 16:11
askolesov
askolesov approved these changes Sep 29, 2022
View reviewed changes
Copy link
Contributor

@askolesov askolesov left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks.

@askolesov
Copy link
Contributor

Does anybody know why my commits don't trigger pipelines?

@askolesov
Copy link
Contributor

Ok, got it, because it's a fork. We need a way to trigger them befor merge.

askolesov
askolesov suggested changes Sep 29, 2022
View reviewed changes
Copy link
Contributor

@askolesov askolesov left a comment

Choose a reason for hiding this comment

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

We need to trigger pipelines

@Toktar Toktar changed the base branch from main to develop September 29, 2022 10:59
@Toktar Toktar changed the base branch from develop to main September 30, 2022 14:51
@Toktar Toktar changed the base branch from main to develop September 30, 2022 15:17
Toktar
Toktar previously approved these changes Sep 30, 2022
View reviewed changes
@ankurdotb ankurdotb changed the title feat!: DeactivateDid transaction to mark DIDs as deactivated feat: DeactivateDid transaction to mark DIDs as deactivated Oct 17, 2022
@ankurdotb ankurdotb changed the title feat: DeactivateDid transaction to mark DIDs as deactivated feat: DeactivateDid transaction to mark DIDs as deactivated [DEV-1774] Oct 17, 2022
@ankurdotb
Copy link
Contributor

Task linked: DEV-1774 Merge DID Deactivation PRs

@askolesov askolesov merged commit 3ce5962 into cheqd:develop Oct 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@askolesov askolesov askolesov approved these changes

@Toktar Toktar Toktar left review comments

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@khornonzon @Tweeddalex @askolesov @ankurdotb @Toktar