24 ms·

Be Nice and Write Stable Code

362 points by jaytaylor 8y ago

gregmac 8y ago
I love the MaxInboundMessageSize example. I've run into that many times. Often there will be a note in the release notes about it, and I know I should read the release notes in detail when I upgrade dependencies, but like many people I don't always. Sometimes it's just laziness or complacency -- especially for "utility" libraries like for compression or encoding -- but other times it's a challenge with release notes: * Each version has each release note published independently (or worse: only on the Releases tab in GitHub, and you have to click to expand to read each) * The release notes are really long or dense, and breaking changes are easily missed There's also worse problems: * The release notes don't actually call out the breaking change (you have to read each ticket in detail) * The release notes just say "Bug fixes" or there are no release notes I think along with the suggestions in this article, library authors should also put effort into making good release notes. This includes realizing sometimes people are using from a couple major versions and/or years ago.
- majewsky 8y ago
  God yes! For the apps that I maintain (and which have users outside my team), I enforce high-quality release notes like you describe. Representative example: https://github.com/sapcc/swift-http-import/blob/master/CHANGELOG.md https://github.com/sapcc/swift-http-import/blob/master/CHANG... (note that this also takes SemVer seriously)
  - y0ghur7_xxx 8y ago
    > I enforce high-quality release notes like you describe. Representative example: https://github.com/sapcc/swift-http-import/blob/master/CHANG.. https://github.com/sapcc/swift-http-import/blob/master/CHANG.... (note that this also takes SemVer seriously) Nitpick: you are not using SemVer. A normal version number MUST take the form X.Y.Z where X, Y, and Z are non-negative integers, and MUST NOT contain leading zeroes. X is the major version, Y is the minor version, and Z is the patch version. Each element MUST increase numerically. For instance: 1.9.0 -> 1.10.0 -> 1.11.0.[1] [1]https://semver.org/#spec-item-2 https://semver.org/#spec-item-2 Some of your version numbers lack the patch version.
    - majewsky 8y ago
      Thanks for the heads-up. Will fix that in future releases.
- CoryG89 8y ago
  While it is a good example, you could also use the same example and conclude that the problem was inadequate tests. SemVer is great, but you can't count on dependencies that you do not control actually adhering to it, either intentionally or unintentionally. The only thing that could have prevented something like this for sure was mentioned: > And while nothing in our early testing sent messages larger than 256k, there were plenty of production instances that did. To me, this was the clear failure; not the fact that some dependency broke semver. Their production system relied on being able to send messages larger than 256k, and their tests did not.
  - pvorb 8y ago
    But isn't it impractical to test every feature of every library you are using? In an ideal world you would have everything tested in isolation as well as integration. But in practice there will always be a corner case that remains untested because you don't know all internals of the libraries you use.
    - CoryG89 8y ago
      If you want to be able to randomly upgrade those dependencies and not have to worry about a breaking change, then yes. Semver is not going to help you there. Server is only going to help you when someone knows they are releasing breaking change. And even then, only if they are nice enough to actually follow the spec. You don't have to test every bit of every dependency you use, but upgrading them without either carefully reviewing the changes or having tests in place for at least critical functionality is asking for something like this to happen eventually.
  - gregmac 8y ago
    While it's easy to say, how far do you go? Do you test every bit of every upstream library you use? The ideal is probably yes, but the reality is this rarely happens. Even with a test, you may not find this. In the IOException example, the author calls out why: > When we upgraded, all our tests passed (because our test fixtures emulated the old behavior and our network was not unstable enough to trigger bad conditions) The only way to catch this type of thing is to emulate the entire network side of things, and that's still only as good as your simulation of the real world. Again, reality is even if you test your upstream to this extent, you're probably mocking a bunch of things, and that may mask something in a way you won't see until possibly production use.
h8liu 8y ago
fwiw, I maintain a website called smallrepo (https://smallrepo.com https://smallrepo.com). It builds go language code together, and maintains an always buildable commit set as a "super repo". If you sync to smallrepo (rather than using go get), it can shield you from many unexpected build breakages.
hackernoon 8y ago
Argh! I love the deprecation example! How elegant! How did I never think of this (or why did I never think to ask)! edit: pasted here – func ListItems(query Query) Items { ListItemsWithLimit(query, 0, 0) } func ListItemsWithLimit(query Query, limit int, offset int) Items { // ... }
- SamuelAdams 8y ago
  Wouldn't it be easier to write optional variables, allowing you to keep the same method name? This results in cleaner code that doesn't break existing usage. For example: func ListItems(query Query, limit int = 0, offset int = 0) Items { // .... }
  - jniedrauer 8y ago
    This only works in languages that support optional variables or overloading.
  - relyio 8y ago
    It doesn't result in a cleaner interface, which is what matters the most (arguably).
  - lmm 8y ago
    It would; unfortunately Java doesn't support that.
denkmoon 8y ago
That headline is a summary of all the things I try to do but fail.
QuinnWilton 8y ago
A lot of people are commenting that SemVer doesn't work, because it's still at the mercy of humans choosing good version numbers. Elm's package manager, elm-package, actually tries to remove humans from the equation, by automatically choosing the next version number, based on a diff of the API and the exported types of a package: https://github.com/elm-lang/elm-package#publishing-updates https://github.com/elm-lang/elm-package#publishing-updates It's not perfect, but it's better than anything else I've seen.
- unrealchild 8y ago
  That’s a pretty neat strategy. Elm really brings a lot of cool (and fun) sugar to dev flows. I’ve been looking for a reason (project) to use it in so I can transition from an enamored fan-person to a true evangelist :)
- User23 8y ago
  Is API compatibility computable in general? My instinct is that it is, but I’ve never seen a theorem.
  - ShaneWilton 8y ago
    I'd imagine it isn't, at least depending on how you define API compatibility, and whether you're only looking at the API interfaces. Imagine two versions of a library that implement the function "add". Version 1: add Int -> Int -> Int add x y = x + y Version 2: add Int -> Int -> Int add x y = x * y Both versions expose the same API interface, but the functions that conform to that interface are semantically different. A stronger type system could probably differentiate between the two functions, but I doubt you could generally compute whether both functions implement the same behavior. Perhaps with some sort of functional extensionality you'd be able to compute compatibility perfectly, but I can't imagine that ever being feasible in practice. That being said, what Elm does offer is still a huge improvement over humans trying to guess whether they made any breaking changes :)
    - toast0 8y ago
laurence-myers 8y ago
SemVer is a social construct, not a contract. It's nice when it applies, but you cannot rely on other developers to adhere to it. One man's bugfix is another man's breaking change. If product A implements a workaround for a bug in product B, but the bug gets fixed in a patch version, it could break product A's code, so it becomes a breaking change. The only way to anticipate these changes is reading the change logs/release notes, and thorough automated regression testing. (Obviously unfeasible for every dependency.) Maybe versions should be a single number, like a build number. It just gets tricky when you have multiple versions out there, each requiring patches.
- azernik 8y ago
  Contracts are social constructs too.
  - jestar_jokin 8y ago
    In programming, type contracts are not. Wouldn't it be nice if we had some sort of infallible static analysis tool that absolutely determines what constitutes a breaking change in your codebase?
- nemothekid 8y ago
  >SemVer is a social construct, not a contract. It's nice when it applies, but you cannot rely on other developers to adhere to it. Whats the solution here? Fuck standards? Imagine if we had that same attitude with regards to HTTP.
  - hinkley 8y ago
    I haven’t figured out how to implement this, but the key observation is that Semver is trying to delineate degrees of substitutability (LSP). I think the right solution here is to build version numbers off of your black box tests. The devil is in the details though. What’s a change to the tests mean exactly? Adding new tests is probably a patch release. Modifying tests demands at least a minor version number, but what makes it a major version number? Deleting tests probably qualifies. Removing assertions probably does too. But now what if the author is bad at tests too? Can I substitute my own? (I could do that right now for regression testing, and in fact I do occasionally for interdepartmental issues).
tabtab 8y ago
Regarding exception handling, letting internal exceptions define external behavior is perhaps a bad idea. The possible exception types can be wide and change over time as new parts or features are added. Example: // pseudo-code qry = new query(sql=theSql, dbConfig=DB_FOO); if (! qry.Execute()) { errMsg = "Something went wrong during your query. "; if (qry.errorExceptionName=="DB_Busy") { errMsg += "The database appears to be busy."; // append more } displayAlert(errMsg); } else { processQuery(qry.resultRows); } Here any fatal errors are caught inside the query object, but details are available if and when you wish to take advantage of them outside the query object. The query object (API) user doesn't have to know all possible exceptions types in order to handle an exception properly (or at least in a good-enough way).
- jdbernard 8y ago
  It also bugged me that he was upset when the behavior they were relying on was an internal detail not included in the API's contract. (Incidentally, the API itself did not change because it was something like func Read(in Reader) error, where error was a parent of all exceptions) That's not incidental. In my opinion the authors of the API were completely fine changing the internal detail of which specific exception type was thrown because their public API never made a guarantee beyond it being an instance of error.
  - tabtab 8y ago
    Future-friendly error-handling can indeed be tricky. I ran into this trying to make a lasting email-sending API. I was hesitant to depend on the API's specific exception types, and so considered mapping them to more general categories, yet still giving details for troubleshooting. // pseudo-code err = new Error(hasError=false); // innocent until proven guilty try { sendEmail(...); catch (e in excptn1, excptn6, excptn7) // dummy names err.hasError=true; err.recipientProblem=true; err.errorType = e; catch (e in excptn2, excptn4, excptn9) err.hasError=true; err.contentProblem=true; err.errorType = e; catch (else) err.hasError=true; err.errorType = e; } ... return(err); One could make an emumerable list of error categories, but in this case I wasn't even sure they were mutually exclusive because it still sends to the rest if one recipient is bad.
jarfil 8y ago
This is all great, but I feel like all these problems could be caught just with properly written tests. If your tests correctly cover the API usage of your code, and I mean both your code complying with the intended API and the API complying with the intended usage, then the implementation behind that API should be totally transparent. No need to check versions, release notes, or any of that, just run the API compliance tests on the new version, and if it works then your code should work too.
- lotyrin 8y ago
  Treating the test suite as your contract and having a well structured + documented test suite also makes SemVer considerations a matter of what tests changed and how did they change (new cases, new features, changes to existing features, removed features, etc.)?
- jakobegger 8y ago
  Relying on tests is naive. Your tests can't cover every case. The article even mentions this -- their tests passed, but it failed in production.
  - crdoconnor 8y ago
    They built tests that explicitly assumed that the library's interface wasn't going to change: >When we upgraded, all our tests passed (because our test fixtures emulated the old behavior) Doing that, upgrading your dependencies and expecting everything to work just because those tests passed? That's naivete. If they'd built decent integration tests that used the actual library (instead of "assume nothing changes" fixtures) and made more of an effort to simulate realistic scenarios then their tests probably would have flagged up most of the issues they had. Alas, this seems to be one of the side effects of following the "test pyramid" "best practice".
    - jakobegger 8y ago
      I wasn't talking about that paragraph, but the following paragraph where they had tests, but they didn't test with large enough packets. Tests can never cover every scenario. They are very useful, and they catch a lot of unexpected regressions. But they're just a part of the puzzle, not a replacement for good development practices. Updating a dependency without bothering to read the release notes because you have tests -- maybe naive is the wrong word, maybe hubris fits better.
timwis 8y ago
It would be helpful to mention that these guidelines -- and semver generally -- don't really apply to applications from what I've read. Versioning applications still feels like a gut feeling.
- Zanni 8y ago
  I'd assume marketing considerations dominate the version number discussion for (consumer) applications--whether you want the release to be seen as the good old whatever you know and trust or as new and improved.
3pt14159 8y ago
This is a great piece. I think my only addition to it is to point out two small insights. 1. SemVer works great in some communities (Ruby) and shit in others (Python). Generally the more computer sciency the community the better I find the SemVer'ing. Python has a bunch of scientists using it, so it's less reliable, even if many of the core libraries follow it pretty well. 2. Apps, plugins, frameworks, and libraries are different things with different SemVer strategies. With a paid app versioning is often a marketing decision. Framework plugins versioning is often a "match the framework to reduce mental burden" decision. Whereas frameworks and libraries I find a much higher adherence to what SemVer strives to do.
asimpletune 8y ago
This is something that can I cannot stress the importance of, yet rarely ever receives the recognition that it should. Being able to delicately and compassionstely work on old code, while still bringing valuable updates is hard. It’s even harder to make your changes look easy and obvious in hindsight. This is something that most engineering organizations are not equipped to recognize and promote as a virtue - it’s sort of hard to explain as it is. If anything, this patience can be considered an enemy to progress. When you see people who do this well, take note.
unrealchild 8y ago
I try to encourage everyone to define a contract, code to that contract, and update the contract when it is no longer accurate. The exact nature of the contract is contextual, sometimes a schema, sometimes a well documented comment header or perhaps a project README.
int_19h 8y ago
> Stop trying to justify your refactoring with the "public but internal" argument. If the language spec says it's public, it's public. Your intentions have nothing to do with it. This is so wrong. APIs are for people, not tools, so intent is primary. When tools are not expressive enough to capture and enforce intent, you document it, but it's still primary. Someone using a "public" API that clearly says "for internal use only" is no different from a person that uses workarounds like reflection or direct memory access, and there is no obligation to keep things working for the.
- bgorman 8y ago
  Making an API public means that people can do whatever they want with it. If you are not sure if you want to allow the API in the future it should not be public. People will always look to do the laziest thing possible which might mean hooking into your "public internal API". Then you will never be able to change it and you will have to maintain it forever.
  - njovin 8y ago
    They can do whatever they want with it but you have no obligation to maintain nor support it if it’s not a documented public API, in my opinion. It’s a bit like a house on a corner with a big front yard. People may cut through the grass to save time but you can’t blame the homeowner when he finally puts up a fence.
    - braythwayt 8y ago
      The question of people using your property falls under the law covering _easements_. https://en.wikipedia.org/wiki/Easement https://en.wikipedia.org/wiki/Easement It’s unlikely that allowing the public to short-cut across your yard would create a prescriptive easement, but there are certain circumstances where allowing a party or the public to cross a parcel of land for a sufficiently long period of time does prevent the property owner from erecting a fence. Although it’s not a hard-and-fast rule, in Ontario certain property owners have paths open to the public most of the time, but close them for at least one day of the year, often Christmas or New Year’s Day. The intent is to prevent access for any one or more continuous years, which in turn prevents an easement from being asserted by any party. Although it’s far from as simple as, “if you allow the public access to your land for one continuous year, you allow it forver,” a certain folklore around this has arisen, and thus the practice.
askmike 8y ago
I'm maintaining an open source project[0] and I'm struggling with using SemVer because my "app" doesn't have a single API but a few: At it's core it's a node app. Though I also include a small web server that wraps around it (and a UI frontend). 1. It allows people to write scripts (js) that receives inputs and passes on events based on an API (the strategy API)[1]. 2. It has extensive configuration[2] that sometimes changes form (the config API). 3. It talks to a number of external services (crypto exchanges), over a "common" protocol called the "exchange wrapper API"[3] (I am ignoring the version of the exchange API being consumed). 4. The wrapped webserver comes with an API (REST + WS)[4]. 5. The "core app" is a chain of plugins, when they change the required config/events also change (usually breaking changes 1 and 4)[5]. I could take all of these components apart (microservice way) and version them separately, but I like the monorepo style I use now where pulling one repo means that everything is working together. Also the fact that (in bug reports) people only have to refer to one version (and when on nightly maybe the git commit if I need more details). But versioning is a mess. [0]: https://gekko.wizb.it/ https://gekko.wizb.it/ [1]: https://gekko.wizb.it/docs/strategies/creating_a_strategy.html https://gekko.wizb.it/docs/strategies/creating_a_strategy.ht... [2]: https://github.com/askmike/gekko/blob/develop/sample-config.js https://github.com/askmike/gekko/blob/develop/sample-config.... [3]: https://gekko.wizb.it/docs/extending/add_an_exchange.html#Gekko-39-s-expectations https://gekko.wizb.it/docs/extending/add_an_exchange.html#Ge... [4]: https://gekko.wizb.it/docs/internals/server_api.html#REST-API https://gekko.wizb.it/docs/internals/server_api.html#REST-AP... [5]: https://gekko.wizb.it/docs/internals/events.html#List-of-events-emitted-by-standard-plugins https://gekko.wizb.it/docs/internals/events.html#List-of-eve... ------- This is not a criticism, I typed this out in the hope that someone can point me in a sane direction (given the discussion on versioning)
- drblast 8y ago
  Your project looks cool. Here's some overly harsh criticism from an old dude in no particular order. The problem I see upon an extremely cursory view of your project is that it's trying very, very hard not to be a sellable product. I'm a good programmer. Why do I want to learn your API/library instead of calling the exchange API's directly? Is this saving me time? Is it saving me time long term, even when your code changes and breaks things? If you had to make this into a single web API and sell access to that as your product, what would it look like? There's your versioning and design answer. Why is the app a "chain of plugins?" If the app breaks when the plugins change then they're not really plugins, are they? Does this design solve a problem or did it just seem like a cool way to do it? Also, anything that relies heavily on configuration to work is a fundamentally broken design in my opinion. Configuration is global state that is hard to change, and the heavy-handed presence of it in a project is usually an indicator that the abstractions are wrong and most of the code probably relies on some hidden state that's really hard to debug unless you're the code author. If there's a legitimate runtime choice you don't want to make for the user, that's a function parameter. If that looks messy, you probably left too many decisions for the user to make. An ideal library is stateless so that the user can handle wrapping it with simpler calls and configuration settings. Make building blocks, not skyscrapers. Sometimes I want to grab all of you young people by the shoulders and shake you until you stop reinventing ever more convoluted ways to do RPC. Finally, take everything I say with a grain of salt because I'm heavily medicated right now.
rtpg 8y ago
Django is my gold standard for this. They have great deprecation policies where they deprecate something in the same release that they add alternatives (allowing for you to fix things up before upgrading Django), they document these changes liberally and offer alternatives, make good use of the warnings system (meaning you can run tests in "deprecated functions not allowed" mode to catch stuff), and generally are careful. I'm still shocked at the number of projects that make breaking changes without first releasing a "support both versions" release that lets people test their changes easily. Especially frustrating when you have really basic environment variable renames that could support the deprecated name as a one-liner so easily. Give people the space to upgrade please!
- sametmax 8y ago
  Same. They are not only very patient, but provide migration documentation and helpers. It's also amazing how long they stayed at 0.96 despite being very stable. Then 1.x for a long time too. It has a cost though: django has a hard time going async since it breaks everything. All in all, the python community has a good culture for this. Even the 2 to 3 migration was given more than a decade to proceed. Yet, i feel like we still get a lot of complains, while in js or ruby land you can break things regularly and fans scream it's fine.
torstenvl 8y ago
Sooo adding a field to a struct is supposedly a non-code-breaking feature (yeah, right) but tweaking a constant isn't? This does not fit with my experience of the world.
- uryga 8y ago
  Could you describe some situations where adding a field broke stuff? I can only imagine that happening if your code cares about the exact binary representation/layout of a struct - so serialization, FFI or just bit-twiddling.
  - torstenvl 8y ago
    Serialization is one, but anything involving casting from one struct to another (e.g., if the Berkeley sockets implementation changed the size of struct in_addr), anything requiring careful management of memory alignment (e.g., SSE), etc. could potentially be broken. Never mind the inevitable reckless coding practices that arise in the wild all the time (someone decides to #define a magic number for allocation purposes instead of using sizeof...). On the other hand, a large number of constants are supposed to be tweakable, to the extent that many are designed to be set at compile time. Anyway. OP article had good points overall, I'm just not sold on some of the specifics.
blablabla123 8y ago
I think the title should rather be "write stable architecture", this is kind of misleading. The code can still be unstable according to this ;)
GlitchMr 8y ago
What counts a breaking change? I would say that strictly speaking anything could be a breaking change if an user is crazy enough, so increasing major version all the time is not particularly useful - I mean, you could do that, but what is the purpose of semver then. Consider a following function. f(arg: String): Output Now, let's say that we change it to be generic. Let's assume that `String` implements `SomeInterface<Output>`. f<T: SomeInterface<U>, U>(arg: T): U An user is crazy and passed an empty generic type list and their code broke as there are now two generic arguments instead of none. f<>("a") Or for a different example, let's say that you want to introduce a new function, `g`, but the user does the following. import yourlibrary.* import otherlibrary.* An user is using the function `g` from `otherlibrary`, and their code doesn't compile anymore due to an ambiguity. I would say it's a minor change, but in theory it's possible for it to be a breaking change. I often had situations like this where something could be breaking, but the code had to be really unusual for it to break (and if it would break, it would be a compilation error).
brian_herman 8y ago
https://sentry.io/welcome/ https://sentry.io/welcome/ The best way I have found to write stable code is to capture all exceptions with sentry and just fix them.
planetmaker 8y ago
It's a bit funny that the title reads "write nice and stable code" - but it's actually a plea for "proper" versioning. Starting with the versioning a few examples are given - and I read it as "this is not good versioning" as it continues with the very basic description of how semantic versioning works. Yet all those examples given above are excellent examples of how semantic versioning works at its best! """ 10. Build metadata MAY be denoted by appending a plus sign (...) Examples: (...) 1.0.0-beta+exp.sha.5114f85. """ And these meta data, referencing used library versions and actual hash of the commit used in the build, are given there - and exactly these meta info on used library versions may help a great deal when it comes to checking bug reports as programme behaviour may differ between versions, but work with any.
jve 8y ago
> But for less intrusive changes, I personally feel like you can make some minor SemVer transgressions provided... Kind of contradicts "Often, it seems that version numbers are incremented by "gut feel" instead of any consistent semantic: "This feels like a minor version update." > The value of MaxBufferSize was adjusted downward to 2048 because we discovered a buffer overflow in a lower level library for any larger buffer size. See issue #4144 Technically it's a major version bump, as I understand it. But security is important, so what should we do here in addition to writing it down as first sentence on release notes? Perhaps having an excuse to potentially break downstream code in the name of security should be OK and well communicated (i.e in readme)?
wintorez 8y ago
Or at least "Be Stable and Write Nice Code"
wool_gather 8y ago
> To that end, these are safe as part of a feature release: > > Adding a field or method to a struct/class/enum/etc. Um, adding fields is a breaking change if you do a binary distribution. > Z is the patch version. Changes to this indicate that internal changes were made, but that no changes (even compatible changes) were made to the API. then, contradicting that > Mark a thing as deprecated as soon as it is considered deprecated, even if that is a patch or minor release. Deprecation should be considered a change to the API. I'm not going to thank you for filling up my build logs with warnings when I pull in a patch update of your library. > Deprecation, after all, is a warning condition, not an error condition. No, not if you use -Werror or equivalent.

362 points by jaytaylor 8y ago

I love the MaxInboundMessageSize example. I've run into that many times. Often there will be a note in the release notes about it, and I know I should read the release notes in detail when I upgrade dependencies, but like many people I don't always. Sometimes it's just laziness or complacency -- especially for "utility" libraries like for compression or encoding -- but other times it's a challenge with release notes: * Each version has each release note published independently (or worse: only on the Releases tab in GitHub, and you have to click to expand to read each) * The release notes are really long or dense, and breaking changes are easily missed There's also worse problems: * The release notes don't actually call out the breaking change (you have to read each ticket in detail) * The release notes just say "Bug fixes" or there are no release notes I think along with the suggestions in this article, library authors should also put effort into making good release notes. This includes realizing sometimes people are using from a couple major versions and/or years ago.

majewsky 8y ago
God yes! For the apps that I maintain (and which have users outside my team), I enforce high-quality release notes like you describe. Representative example: https://github.com/sapcc/swift-http-import/blob/master/CHANGELOG.md https://github.com/sapcc/swift-http-import/blob/master/CHANG... (note that this also takes SemVer seriously)
- y0ghur7_xxx 8y ago
  > I enforce high-quality release notes like you describe. Representative example: https://github.com/sapcc/swift-http-import/blob/master/CHANG.. https://github.com/sapcc/swift-http-import/blob/master/CHANG.... (note that this also takes SemVer seriously) Nitpick: you are not using SemVer. A normal version number MUST take the form X.Y.Z where X, Y, and Z are non-negative integers, and MUST NOT contain leading zeroes. X is the major version, Y is the minor version, and Z is the patch version. Each element MUST increase numerically. For instance: 1.9.0 -> 1.10.0 -> 1.11.0.[1] [1]https://semver.org/#spec-item-2 https://semver.org/#spec-item-2 Some of your version numbers lack the patch version.
  - majewsky 8y ago
    Thanks for the heads-up. Will fix that in future releases.
CoryG89 8y ago
While it is a good example, you could also use the same example and conclude that the problem was inadequate tests. SemVer is great, but you can't count on dependencies that you do not control actually adhering to it, either intentionally or unintentionally. The only thing that could have prevented something like this for sure was mentioned: > And while nothing in our early testing sent messages larger than 256k, there were plenty of production instances that did. To me, this was the clear failure; not the fact that some dependency broke semver. Their production system relied on being able to send messages larger than 256k, and their tests did not.
- pvorb 8y ago
  But isn't it impractical to test every feature of every library you are using? In an ideal world you would have everything tested in isolation as well as integration. But in practice there will always be a corner case that remains untested because you don't know all internals of the libraries you use.
  - CoryG89 8y ago
    If you want to be able to randomly upgrade those dependencies and not have to worry about a breaking change, then yes. Semver is not going to help you there. Server is only going to help you when someone knows they are releasing breaking change. And even then, only if they are nice enough to actually follow the spec. You don't have to test every bit of every dependency you use, but upgrading them without either carefully reviewing the changes or having tests in place for at least critical functionality is asking for something like this to happen eventually.
- gregmac 8y ago
  While it's easy to say, how far do you go? Do you test every bit of every upstream library you use? The ideal is probably yes, but the reality is this rarely happens. Even with a test, you may not find this. In the IOException example, the author calls out why: > When we upgraded, all our tests passed (because our test fixtures emulated the old behavior and our network was not unstable enough to trigger bad conditions) The only way to catch this type of thing is to emulate the entire network side of things, and that's still only as good as your simulation of the real world. Again, reality is even if you test your upstream to this extent, you're probably mocking a bunch of things, and that may mask something in a way you won't see until possibly production use.

h8liu 8y ago

fwiw, I maintain a website called smallrepo (https://smallrepo.com https://smallrepo.com). It builds go language code together, and maintains an always buildable commit set as a "super repo". If you sync to smallrepo (rather than using go get), it can shield you from many unexpected build breakages.

hackernoon 8y ago

Argh! I love the deprecation example! How elegant! How did I never think of this (or why did I never think to ask)! edit: pasted here – func ListItems(query Query) Items { ListItemsWithLimit(query, 0, 0) } func ListItemsWithLimit(query Query, limit int, offset int) Items { // ... }

SamuelAdams 8y ago
Wouldn't it be easier to write optional variables, allowing you to keep the same method name? This results in cleaner code that doesn't break existing usage. For example: func ListItems(query Query, limit int = 0, offset int = 0) Items { // .... }
- jniedrauer 8y ago
  This only works in languages that support optional variables or overloading.
- relyio 8y ago
  It doesn't result in a cleaner interface, which is what matters the most (arguably).
- lmm 8y ago
  It would; unfortunately Java doesn't support that.

denkmoon 8y ago

That headline is a summary of all the things I try to do but fail.

QuinnWilton 8y ago

A lot of people are commenting that SemVer doesn't work, because it's still at the mercy of humans choosing good version numbers. Elm's package manager, elm-package, actually tries to remove humans from the equation, by automatically choosing the next version number, based on a diff of the API and the exported types of a package: https://github.com/elm-lang/elm-package#publishing-updates https://github.com/elm-lang/elm-package#publishing-updates It's not perfect, but it's better than anything else I've seen.

unrealchild 8y ago
That’s a pretty neat strategy. Elm really brings a lot of cool (and fun) sugar to dev flows. I’ve been looking for a reason (project) to use it in so I can transition from an enamored fan-person to a true evangelist :)
User23 8y ago
Is API compatibility computable in general? My instinct is that it is, but I’ve never seen a theorem.
- ShaneWilton 8y ago
  I'd imagine it isn't, at least depending on how you define API compatibility, and whether you're only looking at the API interfaces. Imagine two versions of a library that implement the function "add". Version 1: add Int -> Int -> Int add x y = x + y Version 2: add Int -> Int -> Int add x y = x * y Both versions expose the same API interface, but the functions that conform to that interface are semantically different. A stronger type system could probably differentiate between the two functions, but I doubt you could generally compute whether both functions implement the same behavior. Perhaps with some sort of functional extensionality you'd be able to compute compatibility perfectly, but I can't imagine that ever being feasible in practice. That being said, what Elm does offer is still a huge improvement over humans trying to guess whether they made any breaking changes :)
  - toast0 8y ago

laurence-myers 8y ago

SemVer is a social construct, not a contract. It's nice when it applies, but you cannot rely on other developers to adhere to it. One man's bugfix is another man's breaking change. If product A implements a workaround for a bug in product B, but the bug gets fixed in a patch version, it could break product A's code, so it becomes a breaking change. The only way to anticipate these changes is reading the change logs/release notes, and thorough automated regression testing. (Obviously unfeasible for every dependency.) Maybe versions should be a single number, like a build number. It just gets tricky when you have multiple versions out there, each requiring patches.

azernik 8y ago
Contracts are social constructs too.
- jestar_jokin 8y ago
  In programming, type contracts are not. Wouldn't it be nice if we had some sort of infallible static analysis tool that absolutely determines what constitutes a breaking change in your codebase?
nemothekid 8y ago
>SemVer is a social construct, not a contract. It's nice when it applies, but you cannot rely on other developers to adhere to it. Whats the solution here? Fuck standards? Imagine if we had that same attitude with regards to HTTP.
- hinkley 8y ago
  I haven’t figured out how to implement this, but the key observation is that Semver is trying to delineate degrees of substitutability (LSP). I think the right solution here is to build version numbers off of your black box tests. The devil is in the details though. What’s a change to the tests mean exactly? Adding new tests is probably a patch release. Modifying tests demands at least a minor version number, but what makes it a major version number? Deleting tests probably qualifies. Removing assertions probably does too. But now what if the author is bad at tests too? Can I substitute my own? (I could do that right now for regression testing, and in fact I do occasionally for interdepartmental issues).

tabtab 8y ago

Regarding exception handling, letting internal exceptions define external behavior is perhaps a bad idea. The possible exception types can be wide and change over time as new parts or features are added. Example: // pseudo-code qry = new query(sql=theSql, dbConfig=DB_FOO); if (! qry.Execute()) { errMsg = "Something went wrong during your query. "; if (qry.errorExceptionName=="DB_Busy") { errMsg += "The database appears to be busy."; // append more } displayAlert(errMsg); } else { processQuery(qry.resultRows); } Here any fatal errors are caught inside the query object, but details are available if and when you wish to take advantage of them outside the query object. The query object (API) user doesn't have to know all possible exceptions types in order to handle an exception properly (or at least in a good-enough way).

jdbernard 8y ago
It also bugged me that he was upset when the behavior they were relying on was an internal detail not included in the API's contract. (Incidentally, the API itself did not change because it was something like func Read(in Reader) error, where error was a parent of all exceptions) That's not incidental. In my opinion the authors of the API were completely fine changing the internal detail of which specific exception type was thrown because their public API never made a guarantee beyond it being an instance of error.
- tabtab 8y ago
  Future-friendly error-handling can indeed be tricky. I ran into this trying to make a lasting email-sending API. I was hesitant to depend on the API's specific exception types, and so considered mapping them to more general categories, yet still giving details for troubleshooting. // pseudo-code err = new Error(hasError=false); // innocent until proven guilty try { sendEmail(...); catch (e in excptn1, excptn6, excptn7) // dummy names err.hasError=true; err.recipientProblem=true; err.errorType = e; catch (e in excptn2, excptn4, excptn9) err.hasError=true; err.contentProblem=true; err.errorType = e; catch (else) err.hasError=true; err.errorType = e; } ... return(err); One could make an emumerable list of error categories, but in this case I wasn't even sure they were mutually exclusive because it still sends to the rest if one recipient is bad.

jarfil 8y ago

This is all great, but I feel like all these problems could be caught just with properly written tests. If your tests correctly cover the API usage of your code, and I mean both your code complying with the intended API and the API complying with the intended usage, then the implementation behind that API should be totally transparent. No need to check versions, release notes, or any of that, just run the API compliance tests on the new version, and if it works then your code should work too.

lotyrin 8y ago
Treating the test suite as your contract and having a well structured + documented test suite also makes SemVer considerations a matter of what tests changed and how did they change (new cases, new features, changes to existing features, removed features, etc.)?
jakobegger 8y ago
Relying on tests is naive. Your tests can't cover every case. The article even mentions this -- their tests passed, but it failed in production.
- crdoconnor 8y ago
  They built tests that explicitly assumed that the library's interface wasn't going to change: >When we upgraded, all our tests passed (because our test fixtures emulated the old behavior) Doing that, upgrading your dependencies and expecting everything to work just because those tests passed? That's naivete. If they'd built decent integration tests that used the actual library (instead of "assume nothing changes" fixtures) and made more of an effort to simulate realistic scenarios then their tests probably would have flagged up most of the issues they had. Alas, this seems to be one of the side effects of following the "test pyramid" "best practice".
  - jakobegger 8y ago
    I wasn't talking about that paragraph, but the following paragraph where they had tests, but they didn't test with large enough packets. Tests can never cover every scenario. They are very useful, and they catch a lot of unexpected regressions. But they're just a part of the puzzle, not a replacement for good development practices. Updating a dependency without bothering to read the release notes because you have tests -- maybe naive is the wrong word, maybe hubris fits better.

timwis 8y ago

It would be helpful to mention that these guidelines -- and semver generally -- don't really apply to applications from what I've read. Versioning applications still feels like a gut feeling.

Zanni 8y ago
I'd assume marketing considerations dominate the version number discussion for (consumer) applications--whether you want the release to be seen as the good old whatever you know and trust or as new and improved.

3pt14159 8y ago

This is a great piece. I think my only addition to it is to point out two small insights. 1. SemVer works great in some communities (Ruby) and shit in others (Python). Generally the more computer sciency the community the better I find the SemVer'ing. Python has a bunch of scientists using it, so it's less reliable, even if many of the core libraries follow it pretty well. 2. Apps, plugins, frameworks, and libraries are different things with different SemVer strategies. With a paid app versioning is often a marketing decision. Framework plugins versioning is often a "match the framework to reduce mental burden" decision. Whereas frameworks and libraries I find a much higher adherence to what SemVer strives to do.

asimpletune 8y ago

This is something that can I cannot stress the importance of, yet rarely ever receives the recognition that it should. Being able to delicately and compassionstely work on old code, while still bringing valuable updates is hard. It’s even harder to make your changes look easy and obvious in hindsight. This is something that most engineering organizations are not equipped to recognize and promote as a virtue - it’s sort of hard to explain as it is. If anything, this patience can be considered an enemy to progress. When you see people who do this well, take note.

unrealchild 8y ago

I try to encourage everyone to define a contract, code to that contract, and update the contract when it is no longer accurate. The exact nature of the contract is contextual, sometimes a schema, sometimes a well documented comment header or perhaps a project README.

int_19h 8y ago

> Stop trying to justify your refactoring with the "public but internal" argument. If the language spec says it's public, it's public. Your intentions have nothing to do with it. This is so wrong. APIs are for people, not tools, so intent is primary. When tools are not expressive enough to capture and enforce intent, you document it, but it's still primary. Someone using a "public" API that clearly says "for internal use only" is no different from a person that uses workarounds like reflection or direct memory access, and there is no obligation to keep things working for the.

bgorman 8y ago
Making an API public means that people can do whatever they want with it. If you are not sure if you want to allow the API in the future it should not be public. People will always look to do the laziest thing possible which might mean hooking into your "public internal API". Then you will never be able to change it and you will have to maintain it forever.
- njovin 8y ago
  They can do whatever they want with it but you have no obligation to maintain nor support it if it’s not a documented public API, in my opinion. It’s a bit like a house on a corner with a big front yard. People may cut through the grass to save time but you can’t blame the homeowner when he finally puts up a fence.
  - braythwayt 8y ago
    The question of people using your property falls under the law covering _easements_. https://en.wikipedia.org/wiki/Easement https://en.wikipedia.org/wiki/Easement It’s unlikely that allowing the public to short-cut across your yard would create a prescriptive easement, but there are certain circumstances where allowing a party or the public to cross a parcel of land for a sufficiently long period of time does prevent the property owner from erecting a fence. Although it’s not a hard-and-fast rule, in Ontario certain property owners have paths open to the public most of the time, but close them for at least one day of the year, often Christmas or New Year’s Day. The intent is to prevent access for any one or more continuous years, which in turn prevents an easement from being asserted by any party. Although it’s far from as simple as, “if you allow the public access to your land for one continuous year, you allow it forver,” a certain folklore around this has arisen, and thus the practice.

askmike 8y ago

I'm maintaining an open source project[0] and I'm struggling with using SemVer because my "app" doesn't have a single API but a few: At it's core it's a node app. Though I also include a small web server that wraps around it (and a UI frontend). 1. It allows people to write scripts (js) that receives inputs and passes on events based on an API (the strategy API)[1]. 2. It has extensive configuration[2] that sometimes changes form (the config API). 3. It talks to a number of external services (crypto exchanges), over a "common" protocol called the "exchange wrapper API"[3] (I am ignoring the version of the exchange API being consumed). 4. The wrapped webserver comes with an API (REST + WS)[4]. 5. The "core app" is a chain of plugins, when they change the required config/events also change (usually breaking changes 1 and 4)[5]. I could take all of these components apart (microservice way) and version them separately, but I like the monorepo style I use now where pulling one repo means that everything is working together. Also the fact that (in bug reports) people only have to refer to one version (and when on nightly maybe the git commit if I need more details). But versioning is a mess. [0]: https://gekko.wizb.it/ https://gekko.wizb.it/ [1]: https://gekko.wizb.it/docs/strategies/creating_a_strategy.html https://gekko.wizb.it/docs/strategies/creating_a_strategy.ht... [2]: https://github.com/askmike/gekko/blob/develop/sample-config.js https://github.com/askmike/gekko/blob/develop/sample-config.... [3]: https://gekko.wizb.it/docs/extending/add_an_exchange.html#Gekko-39-s-expectations https://gekko.wizb.it/docs/extending/add_an_exchange.html#Ge... [4]: https://gekko.wizb.it/docs/internals/server_api.html#REST-API https://gekko.wizb.it/docs/internals/server_api.html#REST-AP... [5]: https://gekko.wizb.it/docs/internals/events.html#List-of-events-emitted-by-standard-plugins https://gekko.wizb.it/docs/internals/events.html#List-of-eve... ------- This is not a criticism, I typed this out in the hope that someone can point me in a sane direction (given the discussion on versioning)

drblast 8y ago
Your project looks cool. Here's some overly harsh criticism from an old dude in no particular order. The problem I see upon an extremely cursory view of your project is that it's trying very, very hard not to be a sellable product. I'm a good programmer. Why do I want to learn your API/library instead of calling the exchange API's directly? Is this saving me time? Is it saving me time long term, even when your code changes and breaks things? If you had to make this into a single web API and sell access to that as your product, what would it look like? There's your versioning and design answer. Why is the app a "chain of plugins?" If the app breaks when the plugins change then they're not really plugins, are they? Does this design solve a problem or did it just seem like a cool way to do it? Also, anything that relies heavily on configuration to work is a fundamentally broken design in my opinion. Configuration is global state that is hard to change, and the heavy-handed presence of it in a project is usually an indicator that the abstractions are wrong and most of the code probably relies on some hidden state that's really hard to debug unless you're the code author. If there's a legitimate runtime choice you don't want to make for the user, that's a function parameter. If that looks messy, you probably left too many decisions for the user to make. An ideal library is stateless so that the user can handle wrapping it with simpler calls and configuration settings. Make building blocks, not skyscrapers. Sometimes I want to grab all of you young people by the shoulders and shake you until you stop reinventing ever more convoluted ways to do RPC. Finally, take everything I say with a grain of salt because I'm heavily medicated right now.

rtpg 8y ago

Django is my gold standard for this. They have great deprecation policies where they deprecate something in the same release that they add alternatives (allowing for you to fix things up before upgrading Django), they document these changes liberally and offer alternatives, make good use of the warnings system (meaning you can run tests in "deprecated functions not allowed" mode to catch stuff), and generally are careful. I'm still shocked at the number of projects that make breaking changes without first releasing a "support both versions" release that lets people test their changes easily. Especially frustrating when you have really basic environment variable renames that could support the deprecated name as a one-liner so easily. Give people the space to upgrade please!

sametmax 8y ago
Same. They are not only very patient, but provide migration documentation and helpers. It's also amazing how long they stayed at 0.96 despite being very stable. Then 1.x for a long time too. It has a cost though: django has a hard time going async since it breaks everything. All in all, the python community has a good culture for this. Even the 2 to 3 migration was given more than a decade to proceed. Yet, i feel like we still get a lot of complains, while in js or ruby land you can break things regularly and fans scream it's fine.

torstenvl 8y ago

Sooo adding a field to a struct is supposedly a non-code-breaking feature (yeah, right) but tweaking a constant isn't? This does not fit with my experience of the world.

uryga 8y ago
Could you describe some situations where adding a field broke stuff? I can only imagine that happening if your code cares about the exact binary representation/layout of a struct - so serialization, FFI or just bit-twiddling.
- torstenvl 8y ago
  Serialization is one, but anything involving casting from one struct to another (e.g., if the Berkeley sockets implementation changed the size of struct in_addr), anything requiring careful management of memory alignment (e.g., SSE), etc. could potentially be broken. Never mind the inevitable reckless coding practices that arise in the wild all the time (someone decides to #define a magic number for allocation purposes instead of using sizeof...). On the other hand, a large number of constants are supposed to be tweakable, to the extent that many are designed to be set at compile time. Anyway. OP article had good points overall, I'm just not sold on some of the specifics.

blablabla123 8y ago

I think the title should rather be "write stable architecture", this is kind of misleading. The code can still be unstable according to this ;)

GlitchMr 8y ago

What counts a breaking change? I would say that strictly speaking anything could be a breaking change if an user is crazy enough, so increasing major version all the time is not particularly useful - I mean, you could do that, but what is the purpose of semver then. Consider a following function. f(arg: String): Output Now, let's say that we change it to be generic. Let's assume that `String` implements `SomeInterface<Output>`. f<T: SomeInterface<U>, U>(arg: T): U An user is crazy and passed an empty generic type list and their code broke as there are now two generic arguments instead of none. f<>("a") Or for a different example, let's say that you want to introduce a new function, `g`, but the user does the following. import yourlibrary.* import otherlibrary.* An user is using the function `g` from `otherlibrary`, and their code doesn't compile anymore due to an ambiguity. I would say it's a minor change, but in theory it's possible for it to be a breaking change. I often had situations like this where something could be breaking, but the code had to be really unusual for it to break (and if it would break, it would be a compilation error).

brian_herman 8y ago

https://sentry.io/welcome/ https://sentry.io/welcome/ The best way I have found to write stable code is to capture all exceptions with sentry and just fix them.

planetmaker 8y ago

It's a bit funny that the title reads "write nice and stable code" - but it's actually a plea for "proper" versioning. Starting with the versioning a few examples are given - and I read it as "this is not good versioning" as it continues with the very basic description of how semantic versioning works. Yet all those examples given above are excellent examples of how semantic versioning works at its best! """ 10. Build metadata MAY be denoted by appending a plus sign (...) Examples: (...) 1.0.0-beta+exp.sha.5114f85. """ And these meta data, referencing used library versions and actual hash of the commit used in the build, are given there - and exactly these meta info on used library versions may help a great deal when it comes to checking bug reports as programme behaviour may differ between versions, but work with any.

jve 8y ago

> But for less intrusive changes, I personally feel like you can make some minor SemVer transgressions provided... Kind of contradicts "Often, it seems that version numbers are incremented by "gut feel" instead of any consistent semantic: "This feels like a minor version update." > The value of MaxBufferSize was adjusted downward to 2048 because we discovered a buffer overflow in a lower level library for any larger buffer size. See issue #4144 Technically it's a major version bump, as I understand it. But security is important, so what should we do here in addition to writing it down as first sentence on release notes? Perhaps having an excuse to potentially break downstream code in the name of security should be OK and well communicated (i.e in readme)?

wintorez 8y ago

Or at least "Be Stable and Write Nice Code"

wool_gather 8y ago

> To that end, these are safe as part of a feature release: > > Adding a field or method to a struct/class/enum/etc. Um, adding fields is a breaking change if you do a binary distribution. > Z is the patch version. Changes to this indicate that internal changes were made, but that no changes (even compatible changes) were made to the API. then, contradicting that > Mark a thing as deprecated as soon as it is considered deprecated, even if that is a patch or minor release. Deprecation should be considered a change to the API. I'm not going to thank you for filling up my build logs with warnings when I pull in a patch update of your library. > Deprecation, after all, is a warning condition, not an error condition. No, not if you use -Werror or equivalent.