Skip to content
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.

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

RUM-5992 Support additional properties mutation #2099

Merged
merged 3 commits into from
Nov 5, 2024

Conversation

maxep
Copy link
Member

@maxep maxep commented Oct 30, 2024

What and why?

Additional properties are generated with internal(set) accessor, preventing users from mutating their attributes using event mappers. These changes align with Android and allow additional properties mutation in both Swift and ObjC.

How?

  • Update model generator to make additional properties mutable
  • Fix Swift reserved keyword in generator; following schema change that define a property named protocol.

Review checklist

  • Feature or bugfix MUST have appropriate tests (unit, integration)
  • Make sure each commit and the PR mention the Issue number or JIRA reference
  • Add CHANGELOG entry for user facing changes
  • Add Objective-C interface for public APIs (see our guidelines [internal]) and run make api-surface)

@maxep maxep requested review from a team as code owners October 30, 2024 14:54
@maxep maxep force-pushed the maxep/RUM-5992/mutable-attributes branch from 3633c04 to d18f061 Compare October 30, 2024 15:02
@maxep maxep changed the title RUM-5992 Add support for reserved swift keywords RUM-5992 Support additional properties mutation Oct 30, 2024
@maxep maxep force-pushed the maxep/RUM-5992/mutable-attributes branch 2 times, most recently from ffc0112 to d7b946d Compare November 4, 2024 08:40
@@ -504,3 +506,55 @@ extension SwiftPrinter.Configuration.AccessLevel: CustomStringConvertible {
}
}
}

extension SwiftStruct.Property {
Copy link
Member

Choose a reason for hiding this comment

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

Could we use backticks all the time? This way we don't have to update the list of reserved keywords.

Copy link
Member Author

@maxep maxep Nov 4, 2024

Choose a reason for hiding this comment

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

Could be! Lets see if it becomes problematic, I don't expect much use of these keywords.

@maxep maxep force-pushed the maxep/RUM-5992/mutable-attributes branch from d7b946d to 4db2118 Compare November 4, 2024 13:10
@maxep maxep requested a review from ncreated November 5, 2024 11:14
Copy link
Member

@ncreated ncreated left a comment

Choose a reason for hiding this comment

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

Looks great, the change is on target 🎯!

@maxep maxep force-pushed the maxep/RUM-5992/mutable-attributes branch from 4db2118 to 04e8ef3 Compare November 5, 2024 15:05
@maxep
Copy link
Member Author

maxep commented Nov 5, 2024

/merge

@dd-devflow
Copy link

dd-devflow bot commented Nov 5, 2024

🚂 MergeQueue: waiting for PR to be ready

This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.

Use /merge -c to cancel this operation!

@maxep
Copy link
Member Author

maxep commented Nov 5, 2024

/remove

@dd-devflow
Copy link

dd-devflow bot commented Nov 5, 2024

🚂 Devflow: /remove

@dd-devflow
Copy link

dd-devflow bot commented Nov 5, 2024

⚠️ MergeQueue: This merge request was unqueued

This merge request was unqueued

If you need support, contact us on Slack #devflow!

@maxep
Copy link
Member Author

maxep commented Nov 5, 2024

/merge

@dd-devflow
Copy link

dd-devflow bot commented Nov 5, 2024

🚂 MergeQueue: pull request added to the queue

The median merge time in develop is 29m.

Use /merge -c to cancel this operation!

@maxep maxep deleted the maxep/RUM-5992/mutable-attributes branch November 6, 2024 08:48
@ncreated ncreated mentioned this pull request Nov 14, 2024
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants