-
Notifications
You must be signed in to change notification settings - Fork 17
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
Set up Plausible tracking for RIS #2456
Conversation
portality/lib/plausible.py
Outdated
logger.error(str(e)) | ||
if app.config.get("DOAJENV") == 'dev': | ||
print(f"Plausible Analytics Event: Goal: {goal}, Payload: {props_kwargs}") | ||
else: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's only the if
that is added here. Anything under else
hasn't changed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
under the old googe analytics code we had a file logger specifically for the GA events, rather than splurging them out to print
statements - could you try that approach instead? Even using logger.debug
would be more legit, and wouldn't need the if
statement at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's not really needed, I added that for my local testing and forgot to clean it up, sorry for that. Removing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once again I'm being fussy about print
debug statements :)
portality/lib/plausible.py
Outdated
logger.error(str(e)) | ||
if app.config.get("DOAJENV") == 'dev': | ||
print(f"Plausible Analytics Event: Goal: {goal}, Payload: {props_kwargs}") | ||
else: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
under the old googe analytics code we had a file logger specifically for the GA events, rather than splurging them out to print
statements - could you try that approach instead? Even using logger.debug
would be more legit, and wouldn't need the if
statement at all.
Remove white char to clean up the diff
Set up Plausible tracking for RIS Export
briefly describe the PR here
This PR...
Developer Checklist
Developers should review and confirm each of these items before requesting review
constants
ormessages
filesdates
)url_for
not hard-codeddevelop
Reviewer Checklist
Reviewers should review and confirm each of these items before approval
If there are multiple reviewers, this section should be duplicated for each reviewer
constants
ormessages
filesdates
)url_for
not hard-codeddevelop
Testing
List user test scripts that need to be run
List any non-unit test scripts that need to be run by reviewers
Deployment
What deployment considerations are there? (delete any sections you don't need)
Configuration changes
What configuration changes are included in this PR, and do we need to set specific values for production
Scripts
What scripts need to be run from the PR (e.g. if this is a report generating feature), and when (once, regularly, etc).
Migrations
What migrations need to be run to deploy this
Monitoring
What additional monitoring is required of the application as a result of this feature
New Infrastructure
What new infrastructure does this PR require (e.g. new services that need to run on the back-end).
Continuous Integration
What CI changes are required for this