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

feat: initial revocation service added #317

Conversation

nitin-vavdiya
Copy link
Contributor

@nitin-vavdiya nitin-vavdiya commented Jun 13, 2024

Description

A verifiable credential revocation service is added.
This is a reference implementation of Verifiable Credentials Status List v2021

Why

In the current application implementation, we cannot issue credentials that can be revoked.

What

Revocation service added to support revocable credentials.
The issuer can revoke an issued credential and check the revocation status of issued credentials.

This is ref. implementation of Verifiable Credentials Status List v2021

This is originally developed at Cofinity-X

Thanks, @mustafasalfiti and @PManaras for initial contribution.

Ref:

Pre-review checks

Please ensure to do as many of the following checks as possible, before asking for committer review:

Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

KICS found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

@nitin-vavdiya nitin-vavdiya force-pushed the feat/revocation-service branch from f2b93b3 to 24cead3 Compare June 13, 2024 11:50
@borisrizov-zf
Copy link
Contributor

Merged #318, you'll need to rebase at some point.

Copy link

Quality Gate Passed Quality Gate passed

Issues
33 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.5% Duplication on New Code

See analysis details on SonarCloud

Copy link

@evegufy evegufy left a comment

Choose a reason for hiding this comment

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

Hi @nitin-vavdiya
did you notice that the contribution #317 is much to huge? (max. 1,000 lines of content (including documentation, code, configuration files, and other forms of source code, see https://www.eclipse.org/projects/handbook/#ip-project-content). I don't think it's reasonable that someone would be able to review such a huge PR.

@nitin-vavdiya
Copy link
Contributor Author

Hi @nitin-vavdiya did you notice that the contribution #317 is much to huge? (max. 1,000 lines of content (including documentation, code, configuration files, and other forms of source code, see https://www.eclipse.org/projects/handbook/#ip-project-content). I don't think it's reasonable that someone would be able to review such a huge PR.

Hi @nitin-vavdiya did you notice that the contribution #317 is much to huge? (max. 1,000 lines of content (including documentation, code, configuration files, and other forms of source code, see https://www.eclipse.org/projects/handbook/#ip-project-content). I don't think it's reasonable that someone would be able to review such a huge PR.

Yes, this is a huge PR as this contains the entire revocation application(APIs, test cases, docs etc) if we push partial changes then the test/checks will not pass or not meet the TRGs(specially document and chart related)
We need an IP review/check for this PR.

@evegufy
Copy link

evegufy commented Jun 19, 2024

Hi @nitin-vavdiya did you notice that the contribution #317 is much to huge? (max. 1,000 lines of content (including documentation, code, configuration files, and other forms of source code, see https://www.eclipse.org/projects/handbook/#ip-project-content). I don't think it's reasonable that someone would be able to review such a huge PR.

Hi @nitin-vavdiya did you notice that the contribution #317 is much to huge? (max. 1,000 lines of content (including documentation, code, configuration files, and other forms of source code, see https://www.eclipse.org/projects/handbook/#ip-project-content). I don't think it's reasonable that someone would be able to review such a huge PR.

Yes, this is a huge PR as this contains the entire revocation application(APIs, test cases, docs etc) if we push partial changes then the test/checks will not pass or not meet the TRGs(specially document and chart related) We need an IP review/check for this PR.

Hi @nitin-vavdiya very good that you're aware that you need to request an ad hoc IP review in such a case.
Still, please keep in mind the following in mind, especially for future contributions: the measure of an ad hoc IP review should be used in extraordinary cases when it it is really not possible to remain under 1000 lines. I'd argue that the api implementation and the according tests could have been tailored and therefore contributed in multiple contributions but especially no TRG promotes huge contributions, they actually promote the contrary, for instance the documentation and the helm chart can be contributed in different pull requests, no TRG requires you otherwise.

Hi @borisrizov-zf, as product expert and committer besides @nitin-vavdiya, are you confident in reviewing this PR?

@borisrizov-zf
Copy link
Contributor

@evegufy Yes, I'm aware of the size, but since @nitin-vavdiya is a committer, the 1000 lines limit doesn't apply. The review is ongoing, I anticipate no issues to be honest since this code has been tested and used by Cofinity-X.

@nitin-vavdiya nitin-vavdiya force-pushed the feat/revocation-service branch from 587919d to bb024da Compare August 8, 2024 05:20
@nitin-vavdiya nitin-vavdiya force-pushed the feat/revocation-service branch from 3fde027 to 643493d Compare September 5, 2024 09:57
@nitin-vavdiya nitin-vavdiya marked this pull request as ready for review September 13, 2024 08:41
Copy link

@evegufy evegufy dismissed their stale review September 25, 2024 13:06

limiting review to comments

@evegufy
Copy link

evegufy commented Sep 25, 2024

@evegufy Yes, I'm aware of the size, but since @nitin-vavdiya is a committer, the 1000 lines limit doesn't apply. The review is ongoing, I anticipate no issues to be honest since this code has been tested and used by Cofinity-X.

Hi @nitin-vavdiya @borisrizov-zf as you're contributing the developments to developers which are not committers, I''ll still recommend to initiate an IP review. I explained the process here. Normally the Eclipse Foundation takes of IP reviews quite fast (if it's not in the midst of vacation season), I've seen some processed even within a day.

@nitin-vavdiya
Copy link
Contributor Author

@evegufy Yes, I'm aware of the size, but since @nitin-vavdiya is a committer, the 1000 lines limit doesn't apply. The review is ongoing, I anticipate no issues to be honest since this code has been tested and used by Cofinity-X.

Hi @nitin-vavdiya @borisrizov-zf as you're contributing the developments to developers which are not committers, I''ll still recommend to initiate an IP review. I explained the process here. Normally the Eclipse Foundation takes of IP reviews quite fast (if it's not in the midst of vacation season), I've seen some processed even within a day.

IP check issue created: https://gitlab.eclipse.org/eclipsefdn/emo-team/iplab/-/issues/16321

@nitin-vavdiya nitin-vavdiya changed the title feat: intial revocation service added feat: initial revocation service added Sep 26, 2024
@nitin-vavdiya
Copy link
Contributor Author

Separate issue created to fix Test and verify helm chart workflow: #353

@nitin-vavdiya
Copy link
Contributor Author

@evegufy Yes, I'm aware of the size, but since @nitin-vavdiya is a committer, the 1000 lines limit doesn't apply. The review is ongoing, I anticipate no issues to be honest since this code has been tested and used by Cofinity-X.

Hi @nitin-vavdiya @borisrizov-zf as you're contributing the developments to developers which are not committers, I''ll still recommend to initiate an IP review. I explained the process here. Normally the Eclipse Foundation takes of IP reviews quite fast (if it's not in the midst of vacation season), I've seen some processed even within a day.

IP check issue created: https://gitlab.eclipse.org/eclipsefdn/emo-team/iplab/-/issues/16321

@evegufy
IP check is approved today, Is it okay to merge now?

@evegufy
Copy link

evegufy commented Oct 17, 2024

@evegufy Yes, I'm aware of the size, but since @nitin-vavdiya is a committer, the 1000 lines limit doesn't apply. The review is ongoing, I anticipate no issues to be honest since this code has been tested and used by Cofinity-X.

Hi @nitin-vavdiya @borisrizov-zf as you're contributing the developments to developers which are not committers, I''ll still recommend to initiate an IP review. I explained the process here. Normally the Eclipse Foundation takes of IP reviews quite fast (if it's not in the midst of vacation season), I've seen some processed even within a day.

IP check issue created: https://gitlab.eclipse.org/eclipsefdn/emo-team/iplab/-/issues/16321

@evegufy IP check is approved today, Is it okay to merge now?

sure

@nitin-vavdiya nitin-vavdiya merged commit 1ed0878 into eclipse-tractusx:develop Oct 18, 2024
12 of 13 checks passed
Copy link

🎉 This PR is included in version 1.0.0-develop.5 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants