Skip to content
This repository has been archived by the owner on Jan 15, 2021. It is now read-only.

v1.1.5 adds breaking changes to the Silicon Labs platforms #63

Open
marcuschangarm opened this issue Jan 27, 2016 · 14 comments
Open

v1.1.5 adds breaking changes to the Silicon Labs platforms #63

marcuschangarm opened this issue Jan 27, 2016 · 14 comments
Labels

Comments

@marcuschangarm
Copy link
Contributor

1cf4642

The c++11 extensions should have been a major version increase.

@bogdanm
Copy link
Contributor

bogdanm commented Jan 27, 2016

We enabled c++11 a while ago. At that time, we didn't find any incompatibilities, and since the API themselves didn't change, it made little sense to releae a major number. What kind of breaking changes are you talking about?

@ciarmcom
Copy link
Member

ARM Internal Ref: IOTSFW-1918

@marcuschangarm
Copy link
Contributor Author

The c++11 enablement was in a major version upgrade.

By making this a patch update you are forcing me to change my code, which is against the semantic versioning meaning of patch.

@bogdanm
Copy link
Contributor

bogdanm commented Jan 28, 2016

I'm not sure actually. semver.org has this to say:

Major version X (X.y.z | X > 0) MUST be incremented if any backwards incompatible changes are introduced to the public API. It MAY include minor and patch level changes. Patch and minor version MUST be reset to 0 when major version is incremented.

It talks about the API. The API didn't change, this was actually a bugfix release. If your code used some core-util code that was buggy and it was fixed, it follows logically that your code needs to change too. I'm really not sure though how this should be handled at versioning level.

@marcuschangarm
Copy link
Contributor Author

So you see no problem in forcing changes on our user base like this? Interesting.

@bogdanm
Copy link
Contributor

bogdanm commented Jan 28, 2016

To quote myself:

I'm really not sure though how this should be handled at versioning level.

@marcuschangarm
Copy link
Contributor Author

Breaking changes = major version update

@bogdanm
Copy link
Contributor

bogdanm commented Jan 28, 2016

Did you read the semver.org quote in my previous post carefully? From that perspective, this is not a breaking change, but a bugfix. And again, I don't know how this is should be handled under semver rules.
In this particular case, I'd say that if you were using copy constructors/assignment operators, there's a quite high change that your code won't work properly. I strongly suggest changing it.

@marcuschangarm
Copy link
Contributor Author

The API did change. I can't even compile it.

My point is, I should always be able to update minor and patch version modules and still expect my code to work without changes.

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 28, 2016

@marcuschangarm Fair point. In this case, using newer standard in our modules, shall be considered as breaking change. We should be more careful with this type of updates, as many of our current modules yet not introduced new only c++1x features.

@bogdanm
Copy link
Contributor

bogdanm commented Jan 28, 2016

This is an interesting situation indeed. You were using some functions generated automatically by the compiler; these functions were already implicitly in the interface, even though they were not explicitly declared. And then I broke that by explicitly removing them. You're right, I didn't consider this.
Notwithstanding that, I still strongly recommend fixing your code if it depended on the default copy constructor/assignment operators defined by the compiler. For any non-trivial type, that code probably doesn't do the right thing.
Is there anything we can do to help mitigate this issue?

@marcuschangarm
Copy link
Contributor Author

@bogdanm Thank you for the offer! I got it from here though.

@PrzemekWirkus I wonder if we could have the automatic testing detect when modules break for different major target versions and parent modules within the mbed-drivers dependency tree?

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 22, 2016

@marcuschangarm Shall this be closed?

@marcuschangarm
Copy link
Contributor Author

@0xc0170 why? the major version hasn't been bumped yet and the current version still breaks the efm32gg-stk board.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants