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

chore: unify external logger code #3098

Merged
merged 5 commits into from
Jun 17, 2024
Merged

chore: unify external logger code #3098

merged 5 commits into from
Jun 17, 2024

Conversation

MohamadJaara
Copy link
Member

PR Submission Checklist for internal contributors

  • The PR Title

    • conforms to the style of semantic commits messages¹ supported in Wire's Github Workflow²
    • contains a reference JIRA issue number like SQPIT-764
    • answers the question: If merged, this PR will: ... ³

  • The PR Description

    • is free of optional paragraphs and you have filled the relevant parts to the best of your ability

What's new in this PR?

Issues

dublicated code for external datadog logger

Solutions

move the code to its own source directory and add it to internal app flavours

Needs releases with:

  • GitHub link to other pull request

Testing

build the app and datadog is there

Test Coverage (Optional)

  • I have added automated test to this contribution

How to Test

Briefly describe how this change was tested and if applicable the exact steps taken to verify that it works as expected.

Notes (Optional)

Specify here any other facts that you think are important for this issue.

Attachments (Optional)

Attachments like images, videos, etc. (drag and drop in the text box)

PR Post Submission Checklist for internal contributors (Optional)

  • Wire's Github Workflow has automatically linked the PR to a JIRA issue

PR Post Merge Checklist for internal contributors

  • If any soft of configuration variable was introduced by this PR, it has been added to the relevant documents and the CI jobs have been updated.
References
  1. https://sparkbox.com/foundry/semantic_commit_messages
  2. https://github.com/wireapp/.github#usage
  3. E.g. feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764.

@AndroidBob
Copy link
Collaborator

Build 5291 failed.

@github-actions GitHub Actions
Copy link
Contributor

Built wire-android-staging-release-pr-3098.apk is available for download

@github-actions GitHub Actions
Copy link
Contributor

Built wire-android-dev-debug-pr-3098.apk is available for download

@AndroidBob
Copy link
Collaborator

Build 5292 succeeded.

The build produced the following APK's:

@github-actions GitHub Actions
Copy link
Contributor

Built wire-android-staging-release-pr-3098.apk is available for download

@github-actions GitHub Actions
Copy link
Contributor

Built wire-android-dev-debug-pr-3098.apk is available for download

@AndroidBob
Copy link
Collaborator

Build 5313 succeeded.

The build produced the following APK's:

yamilmedina
yamilmedina reviewed Jun 17, 2024
View reviewed changes
Copy link
Contributor

@yamilmedina yamilmedina left a comment

Choose a reason for hiding this comment

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

Looking fantastic, I added a suggestion about getting the local props and go for reusage 💪

app/build.gradle.kts Outdated Show resolved Hide resolved
@MohamadJaara MohamadJaara requested a review from yamilmedina June 17, 2024 09:11
yamilmedina
yamilmedina approved these changes Jun 17, 2024
View reviewed changes
Copy link
Contributor

@yamilmedina yamilmedina left a comment

Choose a reason for hiding this comment

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

👏 🐇

vitorhugods
vitorhugods reviewed Jun 17, 2024
View reviewed changes
Copy link
Member

@vitorhugods vitorhugods left a comment

Choose a reason for hiding this comment

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

Instead of external-logger-enabled, what about public vs private source sets?

prod and fdroid can use public (no external logger). The other flavours can use private (with external logger).

I'd like to avoid setting a pattern where every different configurable feature will will have its own source set.

I'd also take the opportunity to remove the internal flavour 😅

@github-actions GitHub Actions
Copy link
Contributor

Built wire-android-staging-release-pr-3098.apk is available for download

@github-actions GitHub Actions
Copy link
Contributor

Built wire-android-dev-debug-pr-3098.apk is available for download

@MohamadJaara
Copy link
Member Author

MohamadJaara commented Jun 17, 2024 •
edited
Loading

using private vs public is a nice idea i will give it a try, but for Internal i have another plan to turn it into an edge environment (publish with every push to RC)
@vitorhugods

@AndroidBob
Copy link
Collaborator

Build 5318 failed.

@vitorhugods
Copy link
Member

using private vs public is a nice idea i will give it a try, but for Internal i have another plan to turn it into an edge environment (publish with every push to RC) @vitorhugods

That's a nice idea for internal!

@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@MohamadJaara MohamadJaara requested a review from vitorhugods June 17, 2024 12:35
@github-actions GitHub Actions
Copy link
Contributor

Built wire-android-staging-release-pr-3098.apk is available for download

@github-actions GitHub Actions
Copy link
Contributor

Built wire-android-dev-debug-pr-3098.apk is available for download

@MohamadJaara MohamadJaara added this pull request to the merge queue Jun 17, 2024
Merged via the queue into develop with commit e88f509 Jun 17, 2024
10 checks passed
@MohamadJaara MohamadJaara deleted the feat/unify-external-logger branch June 17, 2024 14:19
MohamadJaara added a commit that referenced this pull request Jun 17, 2024
@MohamadJaara
chore: unify external logger code (#3098)
6c44105 
(cherry picked from commit e88f509)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yamilmedina yamilmedina yamilmedina approved these changes

@vitorhugods vitorhugods vitorhugods approved these changes

Assignees

@MohamadJaara MohamadJaara

Labels
size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@MohamadJaara @AndroidBob @vitorhugods @yamilmedina