Skip to content

Conversation

@CharlesDuboisSAP
Copy link
Contributor

@CharlesDuboisSAP CharlesDuboisSAP commented Dec 17, 2024

Context

https://sap.stackenterprise.co/questions/65172

Users can't create an object with a field named values and make a List or Map of this object.
Solution: rename our internal variable from values to cloudSdkValues to make sure it doesn't collide.

Definition of Done

  • Functionality scope stated & covered
  • Tests cover the scope above
  • Error handling created / updated & covered by the tests above
  • Documentation updated
  • Release notes updated

@CharlesDuboisSAP CharlesDuboisSAP self-assigned this Dec 17, 2024
@CharlesDuboisSAP CharlesDuboisSAP added please merge Request to merge a pull request please review Request to review a pull request labels Dec 17, 2024
@CharlesDuboisSAP CharlesDuboisSAP changed the title [OData] Renamed internal variable value [OData] Renamed internal variable values to cloudSdkValues Dec 17, 2024
Copy link
Member

@MatKuhr MatKuhr left a comment

Choose a reason for hiding this comment

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

Can we apply the same solution to OData v2?

if( values.containsKey("Id") ) {
final Object value = values.remove("Id");
if( cloudSdkValues.containsKey("Id") ) {
final Object value = cloudSdkValues.remove("Id");
Copy link
Member

Choose a reason for hiding this comment

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

The same problem still remains for value, doesn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm creating a new pull request for that as it's a much bigger change

@CharlesDuboisSAP CharlesDuboisSAP merged commit 9eef08a into main Dec 17, 2024
14 checks passed
@CharlesDuboisSAP CharlesDuboisSAP deleted the fix-value branch December 17, 2024 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

please merge Request to merge a pull request please review Request to review a pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants