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

Added rollbar configuration #635

Merged
merged 6 commits into from
Mar 14, 2024
Merged

Added rollbar configuration #635

merged 6 commits into from
Mar 14, 2024

Conversation

balagan73
Copy link
Contributor

#634

  • Added rollbar configuration

@balagan73 balagan73 requested review from AronNovak and amitaibu March 4, 2024 08:20
// Rollbar settings for LIVE and TEST.
if ($pantheon_env == 'live' || $pantheon_env == 'test') {
$config['rollbar.environment'] = $pantheon_site_name . '.' . $pantheon_env;
$config['rollbar.enabled'] = TRUE;
Copy link
Member

Choose a reason for hiding this comment

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

Here we would also have the access token of rollbar right? If so, let's add a placeholder

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. Access token is saved in the module's config file, but there is a setting which disables the module, here we just enable it.

Copy link
Member

@amitaibu amitaibu left a comment

Choose a reason for hiding this comment

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

Let's add a note in the README, about settings Rollbar.

Maybe under ## Prepare for Deploy section or similar

@balagan73 balagan73 requested a review from amitaibu March 5, 2024 08:54
README.md Outdated

### Rollbar configuration

Rollbar will be automatically enabled on test and live environments. The access tokens need to be added and the rollbar configuration yml file should be committed.
Copy link
Member

Choose a reason for hiding this comment

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

Please provide the link /admin/... for the settings

README.md Outdated
@@ -213,6 +213,12 @@ and you can see the test running in the browser.
Check the [DDEV documentation](https://ddev.readthedocs.io/en/latest/users/debugging-profiling/step-debugging/)
if you are using other IDE or want to know more about this feature.

## Prepare for Deployment

### Rollbar configuration
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 move it under Deploy to Pantheon and remove Prepare for Deployment

@balagan73 balagan73 requested a review from amitaibu March 5, 2024 12:19
@balagan73 balagan73 changed the title Added rollbar configuration. Added rollbar configuration Mar 8, 2024
README.md Outdated
@@ -215,6 +215,10 @@ if you are using other IDE or want to know more about this feature.

## Deploy to Pantheon

### Rollbar configuration

Rollbar will be automatically enabled on test and live environments. The access tokens need to be added under '/admin/config/services/rollbar' and the rollbar configuration yml file should be committed.
Copy link
Member

@amitaibu amitaibu Mar 10, 2024

Choose a reason for hiding this comment

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

Actually, instead of /admin/config/services/rollbar - can't we have the setting placeholder on settings.pantheon.php?

@balagan73 balagan73 requested a review from amitaibu March 11, 2024 08:32
@balagan73 balagan73 merged commit c1199a7 into main Mar 14, 2024
1 check passed
@balagan73 balagan73 deleted the 634-rollbar branch March 14, 2024 08:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants