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

Add google analytics sample data #9571

Merged
merged 3 commits into from
Mar 28, 2024

Conversation

AdRiley
Copy link
Member

@AdRiley AdRiley commented Mar 28, 2024

Pull Request Description

This adds a new Google_Credential type of Sample. Which when passed to supporting components (like Google analytics) makes that component produce sample data. This can then be used to publish example workflows.

Sample data is fairly basic right now, but could be extended in the future.

image

Important Notes

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the
    Scala,
    Java,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • All code has been tested:
    • Unit tests have been written where possible.
    • If GUI codebase was changed, the GUI was tested when built using ./run ide build.

@AdRiley AdRiley added the CI: No changelog needed Do not require a changelog entry for this PR. label Mar 28, 2024
Comment on lines +17 to +19
## ICON key
Feeding this into supporting components will give sample data for that commponent.
Sample
Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering if it wouldn't be better to have a separate type (e.g. Google_Sample_Data_Source) and if a method supports the 'sample' mode, it could take credentials : Google_Credential | Google_Sample_Data_Source clearly indicating that either is accepted.

This way, we can easily indicate in the type if the sample mode is available or not.

In the current state, it will only be seen at runtime.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah. It's a fair suggestion. Can't decide if it is better or not. I think generally this will only be used in workflows that we have built as samples so it's probably unlikely that a user will actually build a workflow using it.

Think I'm going to merge this one as is for now.

Copy link
Member

Choose a reason for hiding this comment

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

so it's probably unlikely that a user will actually build a workflow using it.

I guess that suggests that keeping it separate may be a good idea.

Copy link
Member

@radeusgd radeusgd left a comment

Choose a reason for hiding this comment

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

Looks OK.

One idea about the API - I'm concerned if having the Sample available in all kinds of operations we'll have in the future will be that good. But I'm not really sure if it's a problem, so please treat as a light optional suggestion - I'm not 100% convinced my suggestion is really better than the current approach.

@AdRiley AdRiley merged commit c725703 into develop Mar 28, 2024
39 of 42 checks passed
@AdRiley AdRiley deleted the wip/adr/add-google-analytics-sample-data branch March 28, 2024 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: No changelog needed Do not require a changelog entry for this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants