fix(analysis): add compatibility rules class-declaration-abstractness-changed #974

Mamatha1718 · 2024-12-21T17:08:29Z

Title

class-declaration-abstractness-changed #974

See our DEVELOPERS guide below:
https://github.com/accordproject/concerto/blob/main/DEVELOPERS.md

Closes #974

Summary

This pull request introduces abstractness checks for class declarations to enhance compatibility and versioning analysis.

Changes

=> Added logic to compare isAbstract property in class declarations.
=> Retained existing type-change reports for class declarations.
=> Introduced new keys:
=> class-declaration-abstract-to-concrete: Classified as a minor change.
=> class-declaration-concrete-to-abstract: Classified as a major change.
=>Updated rules in compare-config.ts to reflect the above changes.+

Flags

=> Ensure the abstractness change handling aligns with other compatibility rules.
=> Breaking changes introduced by concrete-to-abstract modifications must be validated thoroughly.

Related Issues

Author Checklist

Ensure you provide a DCO sign-off for your commits using the --signoff option of git commit.
Vital features and changes captured in unit and/or integration tests
Commits messages follow AP format
Extend the documentation, if necessary
Merging to main from fork:branchname

DS-AdamMilazzo

I'm sure the Concerto team would like to see something in the PR description.

DS-AdamMilazzo · 2024-12-21T21:34:12Z

packages/concerto-analysis/src/comparers/class-declarations.ts

@@ -54,11 +54,17 @@ const classDeclarationTypeChanged: ComparerFactory = (context) => ({
        if (aType === bType) {
            return;
        }
-        context.report({


The existing type-change report shouldn't be removed. Instead, the if statement above should be inverted (aType !== bType or whatever) and the report should be put in there.

DS-AdamMilazzo · 2024-12-21T21:36:50Z

packages/concerto-analysis/src/comparers/class-declarations.ts

+            const changeSeverity = isAbstract(a) ? 'minor' : 'major';
+            context.report({
+                key: 'class-declaration-abstractness-changed',
+                message: `The class "${a.getName()}" changed from ${changeType} (${changeSeverity} change).`,


The severity can't simply be expressed in the message but must be defined here:

concerto/packages/concerto-analysis/src/compare-config.ts

Line 48 in 85366be

rules: {

You'll need to create two different keys: one for abstract to concrete, and one for concrete to abstract.

I have added the rules, and change the severity to changeKey.
The pull request is ready for review.
please let me know if any changes need.

DS-AdamMilazzo · 2024-12-22T18:01:29Z

It looks okay to me, but I'm not part of the Concerto team so they'll have to review it. I expect they would like to see a test, for example. I think you can search for the other result codes (keys) and find where the existing tests are.

Mamatha1718 · 2024-12-22T18:38:56Z

Thanks for the suggestion! I’ll look into searching for the result codes and check the existing tests.

…

On Sun, Dec 22, 2024, 11:31 PM DS-AdamMilazzo ***@***.***> wrote: It looks okay to me, but I'm not part of the Concerto team so they'll have to review it. I expect they would like to see a test, for example. I think you can search for the other result codes (keys) and find where the existing tests are. — Reply to this email directly, view it on GitHub <#974 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BKJ2ERKOFVNQBUFEJAXYV3D2G35A5AVCNFSM6AAAAABUAZFFHOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKNJYGUZTSNJSHE> . You are receiving this because you authored the thread.Message ID: ***@***.***>

Mamatha1718 · 2024-12-22T18:58:02Z

Hi AdamMilazzo, I hope you're doing well! I’m really interested in contributing to the Accord project, especially for GSoC 2025. I’ve started exploring the codebase and would love to begin contributing by fixing bugs or adding small features. Could you recommend any tasks or open issues that I can start with? I’d also appreciate any advice on how to get more involved early on. How can I connect with you or other mentors to discuss potential project ideas for next year? Additionally, how can I find the listed projects for the Accord organization for GSoC 2025? Also, I was wondering how I can connect with Accord members in the forums or other community channels. Any tips on how to get in touch with other contributors or mentors there would be great! Thank you for your time and support! Looking forward to hearing from you. Best regards, B Mamatha ***@***.*** On Mon, Dec 23, 2024, 12:08 AM Mamatha Bandi ***@***.***> wrote:

…

Thanks for the suggestion! I’ll look into searching for the result codes and check the existing tests. On Sun, Dec 22, 2024, 11:31 PM DS-AdamMilazzo ***@***.***> wrote: > It looks okay to me, but I'm not part of the Concerto team so they'll > have to review it. I expect they would like to see a test, for example. I > think you can search for the other result codes (keys) and find where the > existing tests are. > > — > Reply to this email directly, view it on GitHub > <#974 (comment)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/BKJ2ERKOFVNQBUFEJAXYV3D2G35A5AVCNFSM6AAAAABUAZFFHOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKNJYGUZTSNJSHE> > . > You are receiving this because you authored the thread.Message ID: > ***@***.***> >

DS-AdamMilazzo · 2024-12-22T19:27:54Z

I’m really interested in contributing to the Accord project, especially for GSoC 2025.

Thank you. I'm not part of the Accord/Concerto team, but I'd recommend asking @dselman or @mttrbrts, who are the principals behind it.

sanketshevkar · 2025-01-02T04:57:49Z

Hi @Mamatha1718
Thanks for showing your interest. Can you please update your PR motivations and details in the description box?

I might also want to do a DCO signoff for your commits.

sanketshevkar · 2025-01-02T05:06:03Z

packages/concerto-analysis/src/compare-config.ts

@@ -53,6 +53,8 @@ export const defaultCompareConfig: CompareConfig = {
        'optional-property-added': CompareResult.PATCH,
        'required-property-removed': CompareResult.MAJOR,
        'optional-property-removed': CompareResult.MAJOR,
+        'class-declaration-abstract-to-concrete': CompareResult.MINOR,
+        'class-declaration-concrete-to-abstract': CompareResult.MAJOR, 


How do we decide if the conversions results in a major/minor bump? Is there some sort of modelling convnetion for this?

It's major if existing data can become invalid (i.e. it's not backward-compatible). Otherwise, it's minor or patch. I don't know how they decide between those two. @mttrbrts ?

I agree with @DS-AdamMilazzo, this result makes sense to me.

Mamatha1718 · 2025-01-03T16:10:15Z

Hi @Mamatha1718 Thanks for showing your interest. Can you please update your PR motivations and details in the description box?

I might also want to do a DCO signoff for your commits.
Hi @sanketshevkar,
I change PR description, Will you please let me know any other changes required.
Thank you!

github-actions · 2025-01-19T02:02:45Z

This PR is stale because it has been open 15 days with no activity. Remove stale label or comment or this will be closed in 10 days.

Mamatha1718 · 2025-01-19T04:28:11Z

Hi @sanketshevkar, @DS-AdamMilazzo
I change PR description, Will you please let me know any other changes required.
Thank you!

DS-AdamMilazzo · 2025-01-21T17:50:07Z

Like I said, I'm not part of the Concerto team so I don't really know, but I'm sure @mttrbrts or @dselman know.

Mamatha1718 · 2025-01-22T11:58:39Z

Like I said, I'm not part of the Concerto team so I don't really know, but I'm sure @mttrbrts or @dselman know.

Hi @DS-AdamMilazzo ,Thank you for your response.

mttrbrts

Approved.

@accordproject/maintainers I believe that this should considered a bug fix, and so am comfortable with releasing it in the next minor version release of Concerto.

mttrbrts · 2025-01-22T14:30:57Z

@Mamatha1718 thank you for your first contribution! I'm happy to merge if you can fix the DCO sign-off.

jamieshorten · 2025-01-22T14:37:15Z

@Mamatha1718 this might be helpful for amending the commit messages to contain a sign off message: https://github.com/dcoapp/app#how-it-works

Mamatha1718 · 2025-01-22T18:41:17Z

Hi @mttrbrts Thanks for your response. I signed off DCO.

Mamatha1718 · 2025-01-22T18:43:34Z

Hi @jamieshorten , Thanks for your dcoapp files, that are help me to signoff DCO.
Thank you.

mttrbrts · 2025-01-22T18:43:57Z

Hi @mttrbrts Thanks for your response. I signed off DCO.

There still appears to be an issue here, unfortunately.

You can manually sign-off in the browser here: https://github.com/accordproject/concerto/pull/974/checks?check_run_id=36014157838

Mamatha1718 · 2025-01-23T04:28:24Z

Hi @mttrbrts Thanks for your response. I signed off DCO.

There still appears to be an issue here, unfortunately.

You can manually sign-off in the browser here: https://github.com/accordproject/concerto/pull/974/checks?check_run_id=36014157838

Thank you for pointing that out earlier! I've reviewed the DCO status and successfully resolved the issue. The DCO check now shows a green checkmark, and all commits have been properly signed off.

Please let me know if there's anything else I need to address or if further action is required from my side.
Thank you.

sanketshevkar · 2025-01-23T04:50:51Z

Hi @Mamatha1718
There is a ts-check failing can you please check it?

Mamatha1718 · 2025-01-23T05:57:16Z

Hi @Mamatha1718 There is a ts-check failing can you please check it?

Hi @sanketshevkar , i solve the issue with adding these
interface ClassDeclaration {
isAbstract(): boolean;
getName(): string;
}
const isAbstract = (declaration: ClassDeclaration) => declaration.isAbstract();
will you please check once.
Thank you.

sanketshevkar · 2025-01-23T06:37:41Z

PR build is failing for license check https://github.com/accordproject/concerto/actions/runs/12923154472/job/36041165256?pr=974

Mamatha1718 · 2025-01-23T13:33:57Z

Hi @sanketshevkar ,Thank you for pointing that out. I've removed the er.email file as requested. will you please let me know any changes required.
Thank you.

sanketshevkar · 2025-01-27T06:33:04Z

PR check is still failing. Can you please remove the wq file as well?

Mamatha1718 · 2025-01-27T17:52:56Z

Hi @sanketshevkar ,
Thank you for your response! I have removed the unwanted wq file as requested. Please let me know if there are any other changes needed.
Thanks You!

sanketshevkar · 2025-01-28T03:38:58Z

Here are steps which will help you signoff your commits https://github.com/accordproject/concerto/pull/974/checks?check_run_id=36240929516.

If you follow the instructions given there you should be able to signoff of your old commits.

sanketshevkar · 2025-01-28T03:52:14Z

Also you might have to fix the linting errors as well. You can always try running the build, test and lint check commands in the package json to check if your changes are compatible with all the guardrails kept in place before making the commit.

The PR checks basically does the same thing before we merge the commits.

Thanks for having patience!

Signed-off-by: Mamatha Bandi <[email protected]>

wq Signed-off-by: Mamatha Bandi <[email protected]> Signed-off-by: Mamatha Bandi <[email protected]>

Signed-off-by: Mamatha Bandi <[email protected]> Signed-off-by: Mamatha Bandi <[email protected]>

Signed-off-by: Mamatha Bandi <[email protected]>

DS-AdamMilazzo mentioned this pull request Dec 21, 2024

making a concept abstract or changing its inheritance should be considered a breaking change #937

Open

DS-AdamMilazzo reviewed Dec 21, 2024

View reviewed changes

sanketshevkar reviewed Jan 2, 2025

View reviewed changes

github-actions bot added the Stale label Jan 19, 2025

github-actions bot removed the Stale label Jan 20, 2025

mttrbrts changed the title ~~class-declaration-abstractness-changed~~ feat(analysis): add compatibility rules class-declaration-abstractness-changed Jan 22, 2025

mttrbrts changed the title ~~feat(analysis): add compatibility rules class-declaration-abstractness-changed~~ fix(analysis): add compatibility rules class-declaration-abstractness-changed Jan 22, 2025

mttrbrts approved these changes Jan 22, 2025

View reviewed changes

Mamatha1718 force-pushed the main branch from b358dd9 to b5733c6 Compare January 22, 2025 17:45

Mamatha1718 force-pushed the main branch from f315c28 to a7e1689 Compare January 23, 2025 04:08

Mamatha1718 force-pushed the main branch from c31175d to 6dcf559 Compare January 23, 2025 05:52

Mamatha1718 added 6 commits January 29, 2025 18:01

class-declaration-abstractness-changed

714f5a0

Signed-off-by: Mamatha Bandi <[email protected]>

wq wq class-declaration-abstractness-changed with adding rules

4b88bfa

wq Signed-off-by: Mamatha Bandi <[email protected]> Signed-off-by: Mamatha Bandi <[email protected]>

class-declaration-abstractness-changed with adding rules

9b3e8c9

Signed-off-by: Mamatha Bandi <[email protected]> Signed-off-by: Mamatha Bandi <[email protected]>

run workflow in macos

1fe9eaf

Signed-off-by: Mamatha Bandi <[email protected]>

Fix: Remove unnecessary er.email file

88ccbc5

Signed-off-by: Mamatha Bandi <[email protected]>

Removed wq file causing license check failure

1caa5c4

Signed-off-by: Mamatha Bandi <[email protected]>

Mamatha1718 force-pushed the main branch from 31efd17 to 1caa5c4 Compare January 29, 2025 12:31

ESLint Qoute style

bf96d0d

Signed-off-by: Mamatha Bandi <[email protected]>

Mamatha1718 force-pushed the main branch 2 times, most recently from 4a7a65e to bf96d0d Compare January 31, 2025 17:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(analysis): add compatibility rules class-declaration-abstractness-changed #974

fix(analysis): add compatibility rules class-declaration-abstractness-changed #974

Mamatha1718 commented Dec 21, 2024 •

edited

Loading

DS-AdamMilazzo left a comment

DS-AdamMilazzo Dec 21, 2024

DS-AdamMilazzo Dec 21, 2024

Mamatha1718 Dec 22, 2024 •

edited

Loading

DS-AdamMilazzo commented Dec 22, 2024

Mamatha1718 commented Dec 22, 2024 via email

Mamatha1718 commented Dec 22, 2024 via email

DS-AdamMilazzo commented Dec 22, 2024

sanketshevkar commented Jan 2, 2025

sanketshevkar Jan 2, 2025

DS-AdamMilazzo Jan 2, 2025

mttrbrts Jan 22, 2025

Mamatha1718 commented Jan 3, 2025

github-actions bot commented Jan 19, 2025

Mamatha1718 commented Jan 19, 2025 •

edited

Loading

DS-AdamMilazzo commented Jan 21, 2025

Mamatha1718 commented Jan 22, 2025

mttrbrts left a comment

mttrbrts commented Jan 22, 2025

jamieshorten commented Jan 22, 2025

Mamatha1718 commented Jan 22, 2025

Mamatha1718 commented Jan 22, 2025

mttrbrts commented Jan 22, 2025

Mamatha1718 commented Jan 23, 2025

sanketshevkar commented Jan 23, 2025

Mamatha1718 commented Jan 23, 2025 •

edited

Loading

sanketshevkar commented Jan 23, 2025

Mamatha1718 commented Jan 23, 2025

sanketshevkar commented Jan 27, 2025

Mamatha1718 commented Jan 27, 2025 •

edited

Loading

sanketshevkar commented Jan 28, 2025

sanketshevkar commented Jan 28, 2025

fix(analysis): add compatibility rules class-declaration-abstractness-changed #974

Are you sure you want to change the base?

fix(analysis): add compatibility rules class-declaration-abstractness-changed #974

Conversation

Mamatha1718 commented Dec 21, 2024 • edited Loading

Title

Closes #974

Summary

Changes

Flags

Related Issues

Author Checklist

DS-AdamMilazzo left a comment

Choose a reason for hiding this comment

DS-AdamMilazzo Dec 21, 2024

Choose a reason for hiding this comment

DS-AdamMilazzo Dec 21, 2024

Choose a reason for hiding this comment

Mamatha1718 Dec 22, 2024 • edited Loading

Choose a reason for hiding this comment

DS-AdamMilazzo commented Dec 22, 2024

Mamatha1718 commented Dec 22, 2024 via email

Mamatha1718 commented Dec 22, 2024 via email

DS-AdamMilazzo commented Dec 22, 2024

sanketshevkar commented Jan 2, 2025

sanketshevkar Jan 2, 2025

Choose a reason for hiding this comment

DS-AdamMilazzo Jan 2, 2025

Choose a reason for hiding this comment

mttrbrts Jan 22, 2025

Choose a reason for hiding this comment

Mamatha1718 commented Jan 3, 2025

github-actions bot commented Jan 19, 2025

Mamatha1718 commented Jan 19, 2025 • edited Loading

DS-AdamMilazzo commented Jan 21, 2025

Mamatha1718 commented Jan 22, 2025

mttrbrts left a comment

Choose a reason for hiding this comment

mttrbrts commented Jan 22, 2025

jamieshorten commented Jan 22, 2025

Mamatha1718 commented Jan 22, 2025

Mamatha1718 commented Jan 22, 2025

mttrbrts commented Jan 22, 2025

Mamatha1718 commented Jan 23, 2025

sanketshevkar commented Jan 23, 2025

Mamatha1718 commented Jan 23, 2025 • edited Loading

sanketshevkar commented Jan 23, 2025

Mamatha1718 commented Jan 23, 2025

sanketshevkar commented Jan 27, 2025

Mamatha1718 commented Jan 27, 2025 • edited Loading

sanketshevkar commented Jan 28, 2025

sanketshevkar commented Jan 28, 2025

Mamatha1718 commented Dec 21, 2024 •

edited

Loading

Mamatha1718 Dec 22, 2024 •

edited

Loading

Mamatha1718 commented Jan 19, 2025 •

edited

Loading

Mamatha1718 commented Jan 23, 2025 •

edited

Loading

Mamatha1718 commented Jan 27, 2025 •

edited

Loading