use the newest rollbar.js version, but allow user to select custom one #16
Conversation
| @@ -53,6 +53,7 @@ ftrrtf_rollbar: | |||
| source_map_enabled: true | |||
| code_version: "some version string, such as a version number or git sha" | |||
| guess_uncaught_frames: true | |||
| rollbarjs_version: "rollbar_js_version" // i.e. v1.7, if empty then the newest available version will be used | |||
There was a problem hiding this comment.
you should use # for comment, and maybe "newest" as a default value? what you think?
There was a problem hiding this comment.
👍 for comment, I think that we should allow developer not to pass rollbarjs_version config option, instead of forcing him to fill it with "newest"
filipgolonka
force-pushed
the
rollbar_version
branch
from
a91af49 to
cf03cd5
Compare
|
It can break backward compatibility (I'm not sure) so probably we need to relase it as v1.0 |
filipgolonka
force-pushed
the
rollbar_version
branch
from
cf03cd5 to
7f96079
Compare
|
Yeah, I agree with Filip, we discussed this and if we merge this PR, then we should change major version number to let users know that we're breaking backwards compatibility. |
| @@ -13,14 +13,12 @@ | |||
| "require" : { | |||
| "php" : ">=5.3", | |||
| "symfony/framework-bundle": ">=2.3,<3.0", | |||
| "ftrrtf/rollbar-notifier": "~1.0" | |||
| "ftrrtf/rollbar-notifier": "~1.0", | |||
| "twig/extensions": "^1.1" | |||
There was a problem hiding this comment.
Twig is not a mandatory library (and should not be so), it generally can not be installed in a project (e.g. some API).
There was a problem hiding this comment.
@ftrrtf definitely, but there are tests for it. So I will leave it as suggested package and maybe add it as dev dependency?
|
I've been thinking about such features. But I have not found a good universal solution. Perhaps it makes sense to move towards UMD https://github.com/rollbar/rollbar.js#umd--browserify--requirejs--webpack |
|
@ftrrtf I contacted Rollbar support and they gave me info about v1 script (which contains always the newest version from branch v1.x) |
|
@filipgolonka ie no problems with incompatible versions of the js clients and the snippet (#16 (comment))? |
| @@ -49,6 +49,9 @@ public function getConfigTreeBuilder() | |||
| ->booleanNode('guess_uncaught_frames') | |||
| ->defaultFalse() | |||
| ->end() | |||
| ->scalarNode('rollbarjs_version') | |||
| ->defaultValue('') | |||
There was a problem hiding this comment.
I think it would be better to setup the default version here, instead RollbarExtension::getRollbarJsVersion
(https://github.com/ftrrtf/FtrrtfRollbarBundle/pull/16/files#diff-38c58ba1801f78ea80e449d3547809dbR109)
filipgolonka
force-pushed
the
rollbar_version
branch
from
9502d7b to
db81426
Compare
filipgolonka
force-pushed
the
rollbar_version
branch
from
db81426 to
764ff89
Compare
|
@ftrrtf yes, I check it with rollbarjs version from 1.3 to 1.7 - everytime it is working. |
|
#16 (comment) |
|
Yeap, merging. |
use the newest rollbar.js version, but allow user to select custom one
@ftrrtf @mplachta