Skip to content

Updated bugsnag-php dependency to v3#71

Open
agrzegorzewski wants to merge 3 commits intomasterfrom
upgrade-bugsnag-dep
Open

Updated bugsnag-php dependency to v3#71
agrzegorzewski wants to merge 3 commits intomasterfrom
upgrade-bugsnag-dep

Conversation

@agrzegorzewski
Copy link

Goal

Updated the bugsnag-php dependency to v3.

Changeset

  • Updated bugsnag-php to v3
  • Expanded config options on the Settings page

Testing

All changes tested manually

</th>
<td>
<textarea id="bugsnag_filterfields" name="bugsnag_filterfields" class="regular-text filterfields" style="height: 150px;"><?php echo $this->filterFields; ?></textarea>
<textarea id="bugsnag_redacted_keys" name="bugsnag_redacted_keys" class="regular-text redacted_keys" style="height: 150px;"><?php echo $this->redactedKeys; ?></textarea>
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be worth trying to keep filterfields in some way so that upgrading users won't lose their existing values?

Copy link
Author

Choose a reason for hiding this comment

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

Should we keep the old name under the hood to preserve the filters?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure how the form data is persisted, but I was wondering if we keep the filterfields name here but make it set redacted keys, would that meant that developers don't lose their settings after the upgrade?

Copy link
Contributor

Choose a reason for hiding this comment

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

Or perhaps you always lose settings when you upgrade?

Copy link
Author

Choose a reason for hiding this comment

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

The settings persisted when I reinstalled the plugin, so I assume as long as we keep the same name in the settings view, the value would stay

Comment on lines +141 to 146
try {
require_once $this->relativePath(self::$COMPOSER_AUTOLOADER);
return true;
} catch (Exception $e) {
return false;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the purpose of this change and can you be sure that it's not reducing compatibility? It seems to be assuming use of Composer?

Copy link
Author

Choose a reason for hiding this comment

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

As far as I understand, this does not reduce compatibility, because we don't actually use composer here. We just use the autoload it generates to load all of the dependencies. All of the code the autoloader depends on is shipped alongside the plugin.
In v2 of bugsnag-php there were no dependencies, so we were only copying bugsnag-php from the vendor folder to bundle it with the plugin. Now, that we've updated to v3 we have to ship all of the dependencies anyway, for example Guzzle, so we can rely on the Composer generated autoloader and simplify the logic.

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