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

feat: matomo analytics #3888

Merged
merged 3 commits into from Mar 18, 2021
Merged

feat: matomo analytics #3888

merged 3 commits into from Mar 18, 2021

Conversation

naivekook
Copy link
Contributor

@naivekook naivekook commented Mar 11, 2021 •
edited by teolemon

Description

I don't have permissions to update upstream branch, so created a new one.

Related issues

Related PRs

VaiTon
VaiTon requested changes Mar 11, 2021
View reviewed changes
Copy link
Member

@VaiTon VaiTon left a comment

Choose a reason for hiding this comment

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

I will finish the review later this day

app/src/main/java/openfoodfacts/github/scrachx/openfood/analytics/AnalyticsEvent.kt Outdated Show resolved Hide resolved
app/src/main/java/openfoodfacts/github/scrachx/openfood/analytics/AnalyticsView.kt
package openfoodfacts.github.scrachx.openfood.analytics

enum class AnalyticsView(val path: String) {
Scanner("scanner"),
Copy link
Member

Choose a reason for hiding this comment

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

Let's use the common convention of having enums values as UPPERCASE:
https://stackoverflow.com/questions/3069743/coding-conventions-naming-enums

Copy link
Contributor Author

Choose a reason for hiding this comment

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

np I'll change it, but I think in kotlin we should revisit some coding styles :)
good article https://arturdryomov.dev/posts/kotlin-enum-recipes/

Copy link
Member

Choose a reason for hiding this comment

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

I usually follow Android Studio hints, but yeah we should adopt a style.

@VaiTon VaiTon changed the title Fix/matomo analytics feat: matomo analytics Mar 12, 2021
VaiTon
VaiTon approved these changes Mar 12, 2021
View reviewed changes
Copy link
Member

@VaiTon VaiTon left a comment

Choose a reason for hiding this comment

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

Good job!

@naivekook
Copy link
Contributor Author

@VaiTon can we ignore codefactor in this PR?

@VaiTon
Copy link
Member

VaiTon commented Mar 14, 2021

@naivekook rebase and then good to merge!

@naivekook
Copy link
Contributor Author

@VaiTon done

@VaiTon
Copy link
Member

VaiTon commented Mar 17, 2021

@naivekook sorry but still conflicts :(

@sonarcloud
Copy link

sonarcloud bot commented Mar 18, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 6 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@VaiTon VaiTon merged commit 0952459 into openfoodfacts:develop Mar 18, 2021
@naivekook naivekook deleted the fix/matomo-analytics branch March 19, 2021 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@philippeauriach philippeauriach philippeauriach approved these changes

@VaiTon VaiTon VaiTon approved these changes

Assignees
No one assigned
Labels
analytics
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@naivekook @VaiTon @philippeauriach @teolemon