search

revolunet / angular-google-analytics

Google Analytics tracking for your AngularJS apps
MIT License
652 stars 179 forks source link

Removed all deprecated methods and associated tests #194

Closed justinsa closed 7 years ago

justinsa commented 7 years ago

Deprecated methods are now removed. This will require a major version change since it will break any implementation that is using the existing deprecated methods.

justinsa commented 7 years ago

@Toxantron if you have some time to provide a code review it would be appreciated.

Toxantron commented 7 years ago

The code is fine. They only concern I have is the version number. We should rather take the route of 1.2.x because I assume most users will associate 2.x with angular 2.0 and were are not there yet.

What do you think?

justinsa commented 7 years ago

My reasoning for moving the major version up is that this has the potential to be a fully breaking change. Many developers don't lock on minor version and anyone getting the updated package who is using the deprecated methods still will have some unexpected error messages suddenly appear. Pumping the major version is a reality check to most developers that there are major changes and likely some breaking ones.

Toxantron commented 7 years ago

@justinsa I understand your reasons.

It is more a question of likelihood or affected user-base. Will more users run in the missing method issue or will they expect angular 2.0 compatibility. Since our current user-base is exclusively angular 1.5 I assume looking at for them instead of possible new users is the safer choice.

Keeping that in mind I agree to the 2.0 version, just wanted to point out possible misunderstandings.