#889 optimise dependencies [API-2338] #960
Conversation
|
PR closed by Hazelcast automation as no activity (>3 months). Please reopen with comments, if necessary. Thank you for using Hazelcast and your valuable contributions |
|
Reopen am awaiting feedback |
|
|
|
@emreyigit can you explain what you mean. System.Memory is natively included in the framework as per https://apisof.net/catalog/fd22bb9adc9f2e905e80fbc8eb34b37b starting with net core 2.1 hence it is not required to be added an explicit dependency on the newer frameworks. |
|
Sorry @thompson-tomo. Ignore my previous comment please. I just got your point. I need to verify the changes doesn't break anything. Currently, I have limited time but I've added to my to-do. |
|
No worries, let me know if you have any questions. |
|
PR closed by Hazelcast automation as no activity (>3 months). Please reopen with comments, if necessary. Thank you for using Hazelcast and your valuable contributions |
|
@emreyigit could you re-open |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #960 +/- ##
==========================================
+ Coverage 81.43% 81.45% +0.01%
==========================================
Files 1054 1054
Lines 25420 25420
==========================================
+ Hits 20702 20707 +5
+ Misses 4718 4713 -5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
emreyigit
left a comment
emreyigit
left a comment
There was a problem hiding this comment.
Thank you for your contribution @thompson-tomo, I've left few comments. Also, could you resolve the conflict with master, please?
| if (string.IsNullOrWhiteSpace(attributePath)) | ||
| throw new ArgumentException("No attribute path can be null nor empty.", nameof(attributePaths)); | ||
| #if NETFRAMEWORK | ||
| if (attributePath.Contains("[any]")) |
There was a problem hiding this comment.
Could you revert this change since we target framework and behavior of methods is not same with core?
There was a problem hiding this comment.
I am looking at it but can you show me where we are targeting framework?
|
|
||
| #pragma warning disable MA0048 // File name must match type name | ||
| #define INTERNAL_NULLABLE_ATTRIBUTES | ||
| #if NETSTANDARD2_0 || NETCOREAPP2_0 || NETCOREAPP2_1 || NETCOREAPP2_2 || NET45 || NET451 || NET452 || NET6 || NET461 || NET462 || NET47 || NET471 || NET472 || NET48 |
There was a problem hiding this comment.
Same here and other similar swaps between netstandart2.0 and net framework
There was a problem hiding this comment.
See above about these being controlling compilation and the current frameworks.
|
@emreyigit conflict has been resolved. In regards to framework I am not sure I follow. The package is only compiled for net standard 2.0, 2.1 or net 6+. Net framework works as it will use the highest net standard compiled version compatible hence there is no need for the additional compilation symbols. In fact having them there is actually confusing as even though net47 is a condition in the if, the if is not satisfied given that net47 will use the version compiled for net standard 2.0 which is not a condition for the if. Also I have no idea about the status of the github checks. |
|
@thompson-tomo thank you for your contribution. Github checks doesn't trigger due to lack of permission but anyway, I triggered. I'm going to merge after tests passed. |
emreyigit
changed the title
emreyigit
added
Type: Enhancement
|
Ahh ok @emreyigit no worries. I was just confused that it was showing as failed. |
|
Merged. May I ask your use case if you don't mind? It's just out of curiosity since you mentioned that you aim to have a small build as much as possible. |
3 participants
Closes #889