Skip to content

1 adding dash analytics #2

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
Merged
merged 14 commits into from
Feb 14, 2023
Merged

1 adding dash analytics #2

merged 14 commits into from
Feb 14, 2023

Conversation

eliasyishak
Copy link
Contributor

Adding the initial commit for dash_analytics, a package intended to only be used within Dash tooling to unify analytics across different Dash tools in Google Analytics 4.

@eliasyishak eliasyishak mentioned this pull request Feb 13, 2023
Closed
devoncarew
devoncarew reviewed Feb 13, 2023
View reviewed changes
Copy link
Member

@devoncarew devoncarew left a comment

Choose a reason for hiding this comment

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

I made a pass through with some comments. I generally didn't look at the library implementation; has this already been reviewed? And or, should somebody else review the impl?

@eliasyishak
Copy link
Contributor Author

eliasyishak commented Feb 13, 2023 •
edited
Loading

I made a pass through with some comments. I generally didn't look at the library implementation; has this already been reviewed? And or, should somebody else review the impl?

Yes, for the most part, the implementation has been reviewed at my repo github.com/eliasyishak/dash_analytics – do we need to preserve those conversations somehow in this repo?

@devoncarew
Copy link
Member

Yes, for the most part, the implementation has been reviewed at my repo github.com/eliasyishak/dash_analytics – do we need to preserve those conversations somehow in this repo?

I don't think that's necessary. We could retain the commit history if we wanted to; that would be more useful if there had been multiple authors over time say (I don't think we need to figure out the git commands for that in this case).

@devoncarew devoncarew linked an issue Feb 14, 2023 that may be closed by this pull request
add package:dash_analytics #1
Closed
devoncarew
devoncarew approved these changes Feb 14, 2023
View reviewed changes
Copy link
Member

@devoncarew devoncarew left a comment

Choose a reason for hiding this comment

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

lgtm; feel free to land when you're comfortable with the PR

Do consider separating out the content in the readme a bit as per https://github.com/dart-lang/tools/pull/2/files#r1105023051.

@eliasyishak eliasyishak merged commit ecd5bdd into dart-lang:main Feb 14, 2023
@eliasyishak eliasyishak deleted the 1-adding-dash-analytics branch February 14, 2023 20:00
moffatman added a commit to moffatman/dart-tools that referenced this pull request Apr 23, 2025
@moffatman
Feedback dart-lang#2
09e093c 
- Format generate_trie.dart
- Use path join() in generate_trie.dart
- Add charsUntilAsciiLetter()
- Move all codes to Charcode {}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bwilkerson bwilkerson bwilkerson left review comments

@devoncarew devoncarew devoncarew approved these changes

Assignees
No one assigned
Labels
package:source_maps
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

add package:dash_analytics
4 participants
@eliasyishak @devoncarew @bwilkerson @mosuem