melpa / package-build

Tools for assembling a package archive
https://github.com/melpa/melpa
25 stars 34 forks source link

Improve recipe interface #16

Closed tarsius closed 6 years ago

tarsius commented 6 years ago

The main change is in 34164c01e8f5c13c3c0a26d0abce6b0801d46408. The rest continues my quest to have fewer functions so that it is easier for readers to find the actual api and follow the flow of things.

tarsius commented 6 years ago

I have merged this into Melpa too now.

I think we should wait a few days to see if there are any issues and if not, then we should release this as 2.0. (I am aware that you currently do your open-source work in batches and don't mind waiting for your okay a bit.)

There is an outstanding issue #20 aka https://github.com/cask/cask/issues/426, which could be fixed by either merging https://github.com/cask/cask/pull/429 or the kludge in e2acbd9f5f38286dcbb32015f718f3c94389820d. I think that if the cask pr hasn't been merged by the time we are ready for release we should just include the kludge in the release.

In the long run we should probably provide a function like package-build--package that is more convenient for downstreams, possibly package-build-package (rcp version src-dir dst-dir). But there are at least two issues with that:

tarsius commented 6 years ago

I am looking into the breakage right now.

tarsius commented 6 years ago

@purcell I couldn't really figure out what is going on.

The errors happened in cask exec ecukes run by run-travis.sh. I have commented this out for now because I was not only not able to fix these tests but because I am also under the impression that they did not actually perform any useful tests for five years now.

The errors we got was about some function that I have removed from package-build.el. But that's not the only issue.

I have never used ecukes but it seems to me that what features/step-definitions/melpa-steps.el actually tests is if adding an element to a list and then removing that element from that list again results in the same list as we started with. I don't see what possible errors we are trying to cache with that. The list which is being manipulated doesn't exist anymore - I removed it because it is not useful. So the tests become rather pointless and should be removed. The tests did not actually test whether building a package causes the package to become available and whether removing the package causes it to no longer be there. That would be a somewhat useful thing to test. As far as I can tell it only tested whether adding to and removing something from a list worked as expected - which seems fairly pointless. The only regression that could be caught that way is in the functions doing the adding and removing broke. But I am under the impression that the tests failed miserably at doing that, because what's really strange about this, is that the functions that are being tested were removed five years ago without the tests starting to fail.

I think we should just remove these tests, they seem rather poinless. And then we might also want to remove the whole testing infrastructure that made it possible to run those tests.

The file tests/version-tests.el contains references to symbols that I have recently removed. I haven't touched that yet, but for some reason that doesn't cause anything to fail. Is this dead code?

tarsius commented 6 years ago

I have accidentally pushed a 2.0 tag. It has already made it to Melpa-Stable, so we can't really take it back.

And now I have pushed 2.1 to include 56a0ddb30f53d5bca15b7928089a36e7e80df09c in a release. It is an important bugfix.

tarsius commented 6 years ago

Oh! Piuu, looks like it was actually @purcell who intentionally pushed 2.0.

purcell commented 6 years ago

Oh! Piuu, looks like it was actually @purcell who intentionally pushed 2.0.

Yes, IIRC you asked me to tag a release and I confirmed elsewhere that I had. :-) I probably picked 2.0 as an appropriate version number given the API changes since the previous release.

Nice work sorting all of this out. :tada: