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.

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

Implement Observability #9

Merged
merged 41 commits into from
Feb 21, 2024
Merged

Implement Observability #9

merged 41 commits into from
Feb 21, 2024

Conversation

DevAgani
Copy link
Collaborator

@DevAgani DevAgani commented Feb 8, 2024

This PR

  • adds the Observability event publishing into the GoldenSample app.
  • add a github workflow to build and run tests to ensure that the application remains in the expected state.

In the GoldenSampleApp, we can publish ScreenEvents and UserActionEvents. The events are synonymous to the possible events that happen when using the app, either navigating through screens or performing actions.

@DevAgani DevAgani changed the title Implement Observabilit Implement Observability Feb 8, 2024
tibor-is-back
tibor-is-back previously requested changes Feb 9, 2024
View reviewed changes
Copy link
Collaborator

@tibor-is-back tibor-is-back left a comment

Choose a reason for hiding this comment

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

There are some style issues because SwiftLint is not in the projext. Please update the yaml file so that

  • The Golden sample app scheme is configured
  • The App target contains the SwiftLint script step.
    And then fix the SwiftLint issues!

Tests/accounts-journey/unit-tests/AccountsJourneyObservabilityTests.swift Outdated Show resolved Hide resolved
Tests/accounts-journey/unit-tests/AccountsJourneyObservabilityTests.swift Show resolved Hide resolved
accounts-journey/configuration/account_details/AccountDetailsDesign.swift Outdated Show resolved Hide resolved
accounts-journey/presentation/viewmodel/AccountDetailsViewModel.swift Outdated Show resolved Hide resolved
accounts-journey/presentation/viewmodel/AccountsListViewModel.swift Outdated Show resolved Hide resolved
@DevAgani DevAgani dismissed tibor-is-back’s stale review February 14, 2024 07:48

I've updated the PR with some of the changes

@arthurpalves
Copy link
Member

Ideally this needs to follow the approaches stated in this internal initiative. It has been reviewed and agreed upon by both RnD and CS.

arthurpalves
arthurpalves previously requested changes Feb 20, 2024
View reviewed changes
Copy link
Member

@arthurpalves arthurpalves left a comment

Choose a reason for hiding this comment

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

Just some small comments

accounts-journey/Extensions/O11YEvents.swift Outdated Show resolved Hide resolved
accounts-journey/Extensions/O11YEvents.swift Outdated Show resolved Hide resolved
Tests/accounts-journey/unit-tests/AccountsJourneyObservabilityTests.swift Show resolved Hide resolved
@DevAgani DevAgani dismissed arthurpalves’s stale review February 20, 2024 16:07

Changes have been implemented

@DevAgani DevAgani merged commit ad382c6 into main Feb 21, 2024
1 check passed
@DevAgani DevAgani deleted the feat/add-observability branch February 21, 2024 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arthurpalves arthurpalves arthurpalves approved these changes

@tibor-is-back tibor-is-back Awaiting requested review from tibor-is-back tibor-is-back is a code owner

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@DevAgani @arthurpalves @tibor-is-back