Conversation
|
|
In the info.py module, following the guidance provided in the comments for the settings_target property, this commit focuses on refining the handling of settings_target within various package methods. When populating the settings_target property via the package_id() method, the adjustment guarantees that it remains accessible to both the info and original_info methods. By extending its accessibility to these methods, it allows for seamless utilization within the compatibility() method as well. Signed-off-by: Adnan Ali <m_adnanali_1@hotmail.com>
adnan-ali1
force-pushed
the
feature/14527-settings_target_in_compatibility
branch
from
August 21, 2023 11:43
f46ad31 to
1a20b3b
Compare
Signed-off-by: Adnan Ali <m_adnanali_1@hotmail.com>
memsharded
reviewed
Aug 23, 2023
memsharded
left a comment
Member
There was a problem hiding this comment.
Thanks very much for the contribution @adnan-ali1
I think this is looking good overall, but it might still need some improvement and some other extra tests, please let us know if the comments are enough or you'd like some help with that.
Contributor
Author
|
@memsharded thanks for the review. I will update the PR with the proposed changes |
Prevents the crash as in case a requirements is not build as a build_require it does not even has a settings_target and settings some property in null object would crash. Update test cases to check for multiple scenerios: Signed-off-by: Adnan Ali <m_adnanali_1@hotmail.com>
take in the suggestion. Co-authored-by: James <memsharded@gmail.com>
memsharded
approved these changes
Aug 31, 2023
memsharded
left a comment
Member
There was a problem hiding this comment.
Changes are good, and tests are clean and comprehensive, great job, thanks very much!
AbrilRBS
approved these changes
Aug 31, 2023
AbrilRBS
left a comment
Member
There was a problem hiding this comment.
@adnan-ali1 thanks a lot for your contribution, it's really appreciated!
I like the simplicity of adding this feature and the tests you provided are quite comprehensive :)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changelog: Feature: Allow access to
settings_targetin compatibility method.Docs: Omit
Closes #14527
In the info.py module, following the guidance provided in the comments for the settings_target property, this commit focuses on refining the handling of settings_target within various package methods.
When populating the settings_target property via the package_id() method, the adjustment guarantees that it remains accessible to both the info and original_info methods. By extending its accessibility to these methods, it allows for seamless utilization within the compatibility() method as well.
developbranch, documenting this one.Test Case Result
Before
After