Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Breaking change in patch level release #14

Open
sol opened this issue Apr 23, 2015 · 11 comments
Open

Breaking change in patch level release #14

sol opened this issue Apr 23, 2015 · 11 comments

Comments

@sol
Copy link

sol commented Apr 23, 2015

I just realized that the semantics of splitDirectories changed between system-filepath-0.4.13.1 and system-filepath-0.4.13.2.

ghci> splitDirectories "foo/bar/baz"
Loading package system-filepath-0.4.13 ... linking ... done.
[FilePath "foo",FilePath "bar",FilePath "baz"]
ghci> splitDirectories "foo/bar/baz"
Loading package system-filepath-0.4.13.2 ... linking ... done.
[FilePath "foo/",FilePath "bar/",FilePath "baz"]

WTF!

@sol
Copy link
Author

sol commented Apr 23, 2015

718ba6f

@sol
Copy link
Author

sol commented Apr 23, 2015

Not sure what to do about this, but patch level is not even detectable with CPP...

@snoyberg
Copy link
Member

That change was intended to be a bug fix, and only correct incorrect behavior. Can you give me an example of this change causing breakage?

@sol
Copy link
Author

sol commented Apr 23, 2015

Consider e.g.

isBoring :: FilePath -> Bool
isBoring p = ".git/" `elem` dirs || "dist/" `elem` dirs
  where
    dirs = splitDirectories p

vs

isBoring :: FilePath -> Bool
isBoring p = ".git" `elem` dirs || "dist" `elem` dirs
  where
    dirs = splitDirectories p

The first works for system-filepath-0.4.13.2 and newer, the second worked with earlier versions, but will not work as expected with system-filepath-0.4.13.2 and newer.

@snoyberg
Copy link
Member

Thanks, that's a good counterexample. It looks like our best bet is to revert the change and release a new patch release, would you concur?

@sol
Copy link
Author

sol commented Apr 24, 2015

0.4.13.2 has been out for a couple of months already. I have no picture on how much code depends on the new behavior and how much code depends on the old one.

If we stick with the current behavior we should probably deprecate all versions < 0.4.13.2, if we revert the change we should deprecated 0.4.13.2 and 0.4.13.3.

BTW, is there some discussion that motivated this change?

@snoyberg
Copy link
Member

Issue #3 covers this.

There are unfortunately a lot of problems in the library around how it handles issues of this nature. Someone from the FP Complete team will be putting up a public issue and/or blog post about some proposed changes soon, which will hopefully address them all by simplifying the library.

@sol
Copy link
Author

sol commented Apr 25, 2015

I'm fine with either way.

Personally, I just refrain from changing code at all in patch level releases, not even bug fixes, for the simple reason that you can't CPP on patch level.

For the specific case of splitDirectories it's of course valid to challenge current semantics and change it. But this is a breaking change that will effect user code.

In https://github.com/hspec/autospec I currently rely on the system-filepath-0.4.13.2 semantics and I added a lower bound, so I'm covered.

@sol
Copy link
Author

sol commented Apr 25, 2015

Going forward, an other option would be to deprecate 0.4.13.2 and 0.4.13.3 and release current master as 0.5.0.

@snoyberg
Copy link
Member

Note that I've just announced that these libraries will be deprecated in favor of filepath and directory: https://plus.google.com/+MichaelSnoyman/posts/Ft5hnPqpgEx

@sol
Copy link
Author

sol commented May 12, 2015

I think that's the right call.

Sent from my iPhone

On 12 May 2015, at 3:42 pm, Michael Snoyman [email protected] wrote:

Note that I've just announced that these libraries will be deprecated in favor of filepath and directory: https://plus.google.com/+MichaelSnoyman/posts/Ft5hnPqpgEx


Reply to this email directly or view it on GitHub.

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

No branches or pull requests

2 participants