Skip to content

Codebaseresource features mapping with commoncode.resource #1585 #1638

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
Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Codebaseresource features mapping with commoncode.resource #1585 #1638

wants to merge 5 commits into from

Conversation

abanoub-samy-farhan
Copy link

@abanoub-samy-farhan abanoub-samy-farhan commented Mar 22, 2025 •
edited
Loading

Fixes #1585

Tasks

  • Reading the Contribution Guide
  • Tests Passed - All the tests are passing included the new added tests
  • Commits are in uniquely-named feature branch and has no merge conflicts
  • Added ancestors method to the CodeBaseResource model and tested it
  • Added serialize method to the CodebaseResource model and tested it

@abanoub-samy-farhan abanoub-samy-farhan force-pushed the codebaseresource-features-mapping-with-commoncoderesource branch from 2d10d52 to 1fd7e65 Compare March 22, 2025 16:43
@abanoub-samy-farhan abanoub-samy-farhan changed the title Codebaseresource features mapping with commoncoderesource FIX ISSUE #1585: Codebaseresource features mapping with commoncoderesource Mar 22, 2025
@abanoub-samy-farhan abanoub-samy-farhan force-pushed the codebaseresource-features-mapping-with-commoncoderesource branch from 27eb239 to fc85c21 Compare March 24, 2025 18:20
@abanoub-samy-farhan
Copy link
Author

Hi @AyanSinhaMahapatra , I have made some changes to the CodeBaseResource model. I hope you can review them if there are any further changes to be made.

@abanoub-samy-farhan
Copy link
Author

Hello @tdruez, Can you please review the changes I have made and tell me if it require any further imporvements ?

@AyanSinhaMahapatra AyanSinhaMahapatra changed the title FIX ISSUE #1585: Codebaseresource features mapping with commoncoderesource Codebaseresource features mapping with commoncoderesource #1585 Apr 23, 2025
@AyanSinhaMahapatra AyanSinhaMahapatra changed the title Codebaseresource features mapping with commoncoderesource #1585 Codebaseresource features mapping with commoncode.resource #1585 Apr 23, 2025
pombredanne
pombredanne reviewed Apr 25, 2025
View reviewed changes
scanpipe/models.py Outdated
"""
if not self.has_parent():
return []
anscesotrs = deque()
Copy link
Member

Choose a reason for hiding this comment

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

Minor typo:

Suggested change
anscesotrs = deque()
ancestors = deque()

pombredanne
pombredanne reviewed Apr 25, 2025
View reviewed changes
scanpipe/models.py Outdated
that can be serialized to JSON, YAML, etc. it can be used to reconstruct
the resource
"""
serializable = defaultdict(dict)
Copy link
Member

Choose a reason for hiding this comment

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

Why two nested dicts and a defaultdict?

pombredanne
pombredanne reviewed Apr 25, 2025
View reviewed changes
scanpipe/models.py Outdated
@@ -3059,6 +3078,19 @@ def as_spdx(self):
types=self.get_spdx_types(),
)

def serialize(self):
Copy link
Member

Choose a reason for hiding this comment

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

Do you really need this function? or is this used only for tests? If so move if there.

Copy link
Author

Choose a reason for hiding this comment

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

I don't think so after second considerations, even in testing so I removed it from both sides.

@abanoub-samy-farhan
Copy link
Author

Thanks for your feedback @pombredanne , I have reviewed the code and removed the unnecessary serialize function. Kindly, I also want to know if there are any functionalities that should be mapped from commoncode.resource that haven't been implemented yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pombredanne pombredanne Awaiting requested review from pombredanne

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

Successfully merging this pull request may close these issues.

CodebaseResource does not have all the features of commoncode Resource model
2 participants
@abanoub-samy-farhan @pombredanne