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

Composer Package #31

Closed
joseym opened this issue May 25, 2012 · 34 comments
Closed

Composer Package #31

joseym opened this issue May 25, 2012 · 34 comments

Comments

@joseym
Copy link

joseym commented May 25, 2012

Would be awesome if this were available as a composer package.

@javiereguiluz
Copy link

I'd also like to use php-markdown library as a composer package. Currently, the only available package is a stripped port of php-markdown: http://packagist.org/packages/dflydev/markdown

Creating a package is as easy as adding the following composer.json file in your project and submitting it to http://packagist.org website:

{
    "name": "michelf/php-markdown",
    "type": "library",
    "description": "PHP Markdown",
    "homepage": "http://michelf.com/projects/php-markdown",
    "keywords": ["markdown"],
    "license": "BSD-3-Clause",
    "authors": [
        {
            "name": "Michel Fortin",
            "homepage": "http://michelf.com"
        },
        {
            "name": "John Gruber",
            "homepage": "http://daringfireball.net"
        }
    ],
    "require": {
        "php": ">=4.0.5"
    },
    "extra": {
        "branch-alias": {
            "dev-master": "1.0-dev"
        }
    }
}

baldurrensch pushed a commit to baldurrensch/php-markdown that referenced this issue Aug 21, 2012
@sandeepshetty
Copy link

+1 for making this a composer package. In the meantime here is how I use it with composer:

{
    "repositories": [
        {
            "type": "package",
            "package": {
                "name": "michelf/php-markdown-extra",
                "version": "1.2.5",
                "dist": {
                    "url": "http://littoral.michelf.ca/code/php-markdown/php-markdown-extra-1.2.5.zip",
                    "type": "zip"
                }
            }
        }
    ],
    "require": {
        "michelf/php-markdown-extra": "1.2.*"
    }
}

@mauris
Copy link

mauris commented Dec 18, 2012

@michelf Michel please take a look this issue. Over at dflydev's "fork" of your markdown classes, @simensen has done a great job in providing Composer integration and PSR standards compliance.

I am proposing new changes to bring development between dflydev/dflydev-markdown and michelf/php-markdown together and provide ease of integration into end-user projects through Composer.

I urge @michelf to take a look at the lengthy discussion developers have made about helping your markdown classes to be a great plugin and library for reusability. See dflydev/dflydev-markdown#5

@ghost
Copy link

ghost commented Dec 18, 2012

It's good except for the strange use of lower-case. But yes, with @michelf
rather delaying that is a simple task, others are just going to pick up the
ball on this.... and have it seems.

@mauris
Copy link

mauris commented Dec 18, 2012

@Drak - take a look at my pull request. I've made changes to ensure full PSR-0 and PSR-1 compliance.

If @michelf doesn't respond by 22 Dec 2012, I'd make a fork and merge the two projects into one.

@michelf
Copy link
Owner

michelf commented Dec 18, 2012

@thephpdeveloper I have somme difficulty discussing what you're proposing at dflydev/dflydev-markdown#5. It seems like a grab bag request, encompassing many different things should probably be discussed independently one of another. For instance, I'm not going to switch to PHPUnit (I'm currently using MDTest).

@dark Making it a composer package might be a simple task, but I want to familiarize myself with how that works first. I'm the one who'll get support requests by email if things don't work right for someone. Also, I'd rather wait a little to make sure there's nothing I want to change in the packaging of the lib branch before offering everyone to switch to it by making it a composer package.

@mauris
Copy link

mauris commented Dec 18, 2012

@michelf I'm very glad that you responded. I began to wonder if you had abandoned this project as this issue #31 was left hanging with no response from you for 7 months. Allow me to share with you there is way more than just PHPUnit you're missing out.

  • Travis CI: continuous integration and automated testing. With this power of automation, I recommend you include your MDTest scripts and run it with Travis CI
  • Composer: dependency manager for better release control and packaging for PHP
  • Packagist: library submitted to Packagist for public listing so that developers can easily use Markdown parser by adding just one line into their Composer dependency file composer.json
  • Namespace: You'll have better namespace management
  • Object Oriented Abstraction: an interface would do great as it helps to decouple

You should look at this as a 2.0 release and channel your support request to Github Repository Issues so that more developers can participate and help you along the way.

The point about Open Source Software Development is not to just show your source code. OSSD is about people coming together to solve a problem and that more and more people can come in and sustain the project together. That's how OSS works.

If you need help on Composer, you can take a look at the slides I've created and presented over at SpeakerDeck: https://speakerdeck.com/mauris/introduction-to-composer-dependency-manager-for-php

Yes current users of your legacy markdown classes may not be able to receive updates when we upgrade, but it's time to let go and move on. We need to make great libraries like what you @michelf have created sustainable and friendly for developers.

I have watched many libraries die off because no developers were able to update them and upgrade processes were tedious. Composer has already made it much easier for developers to update their project dependencies already, so why re-invent that wheel.

I sincerely hope that this will help you better to understand the concern I have for this great project.

@mauris
Copy link

mauris commented Dec 18, 2012

By the way, I couldn't find documentation for your MDTest. While MDTest may be more suitable to test Markdown processing, other developers may not be able to access your custom test. Providing a PHPUnit test may offer some comfort and better stability.

Please enlighten me if you do have documentation for MDTest. Thank you.

@michelf
Copy link
Owner

michelf commented Dec 18, 2012

@thephpdeveloper https://github.com/michelf/mdtest

Also, many of the things you're suggesting are already implemented one way or another, are already in the works, or being planned. Look at the various open issues around here to get an idea. I'm sorry if things are moving slowly, but I have very little time to devote to PHP Markdown these days.

If you have other improvements to suggest, please open separate issues or pull requests.

@joseym
Copy link
Author

joseym commented Dec 18, 2012

@michelf

I'm the one who'll get support requests by email if things don't work right for someone. Also, I'd rather wait a little to make sure there's nothing I want to change in the packaging of the lib branch before offering everyone to switch to it by making it a composer package.

The great thing about composer classes is after you add the composer.json file, you submit the current repo to packages. They give you a key that you plug into github and then any changes you push are automatically reflected in the composer package.

You can define which branch, tag, hash, whatever you want composer to use so if you think you'll be branching off in a different direction you're able to tell the package which direction you're currently supporting.

Then for anyone to use your classes they just run php composer.phar update. All it really does is allow your project to be automatically included in other developers work.

@mauris
Copy link

mauris commented Dec 18, 2012

@michelf with composer, you no longer have to manually zip up and upload your zip file for others to download. They can download easily through Composer, and it provides updates from your git repository through version tagging

@joseym
Copy link
Author

joseym commented Dec 18, 2012

@michelf I think what @thephpdeveloper and I are trying to say is that implementing composer doesn't mean you have to manage extra code or repos, it's runs behind the scenes and makes code distribution easier and more conventional. You can still manage this github repo, create tags and distribute the handy github archive links all the while packagist is tracking the project and ensuring that whatever version you've linked it to stays up to date so developers embracing composer can integrate your project into theirs without any hassle or additiona code management.

Cloning is sloppy, submodules have their place but are also fairly sloppy. Both require effort to ensure they stay updated with newest versions. Composer takes that effort away.

@mauris
Copy link

mauris commented Dec 18, 2012

@joseym indeed.

@michelf We're trying to help you automate things so that you don't have to manage this repository so much. Let the OSSD ecosystem take charge.

@michelf
Copy link
Owner

michelf commented Dec 19, 2012

@joseym @thephpdeveloper A composer package is going to be part of the next release. That was decided about 3 months ago and was part of my motivation for starting the lib branch, along with namespace support, autoloading, etc. I'm currently focussing on a couple of issues in the parser, next will be fine tuning the package (including composer), then deciding on the deprecation policy for the older master & extra branches (I'll still be keeping them in sync for at least a year, be warned).

As for the composer file, I want to review the available metadata and its documentation before including it. This is why I haven't merged pull #31 yet, but I'll get to it.

@michelf
Copy link
Owner

michelf commented Jan 7, 2013

Reading the documentation, one thing is bothering me about Packagist: it fetches all the tags in a repository and tries to create a package from each. There are currently many tags for non-Lib versions of PHP Markdown in this repository, what will it do with these? I also planned to use the 'l' prefix before Lib-branch versions (as I'm using the 'v' prefix for the master branch and the 'x' prefix for extra), but that won't work: according to its documentation, Packagist wants either 'v' or no prefix. I'd like if there was a way to fix that... or alternatively to map versions numbers with tags manually. Someone has a tip?

@simensen
Copy link
Contributor

simensen commented Jan 7, 2013

There are currently many tags for non-Lib versions of PHP Markdown in this repository, what will it do with these?

Composer will only create packages from branches and tags that have a composer.json in them. So in your case any existing tags will be skipped.

I also planned to use the 'l' prefix before Lib-branch versions (as I'm using the 'v' prefix for the master branch and the 'x' prefix for extra)

I do not think that Composer supports alternate prefixes for automatically picking up tags. I could be wrong on that but I'm pretty confident that this does not exist. Yet.

It would probably be possible to add a "custom prefix" configuration option (maybe via extra) that would override v to l. I doubt that the Composer team would put a very high priority on this, though. If they were not directly opposed to the idea I'd probably be willing to look into adding it myself.

I hope that whatever solution you land on is very Composer versioning friendly. Composer pushes semantic versioning pretty hard and a healthy chunk of the projects on Packagist follow it. I know that you have a very specific way of doing things but if you can make it work with semantic versioning it will make it extremely useful for people relying on your PHP Markdown library.

I'm not sure if you intend to maintain the lib branch separately from the master branch indefinitely but if you are using the v prefix for master maybe you can just move the lib stuff to master? (I had to ask — it would solve part of the problem ;) )

Also, I noticed early on you have letter suffixes. I'm not exactly sure what those are but I'm guessing that v1.0.1d is more or less v1.0.1.4? Composer is going to have a hard time with the suffixes as they won't mean anything. Are you going to be OK switching to strict dotted numbers?


If you don't change your versioning it will still "work." If you tagged l1.2.6b someone would be able to require {'michelf/php-markdown': 'l1.2.6b'} but that would lock users of that library to that exact version of PHP Markdown. This means that if an application requires package A and package B, and package A requires {'michelf/php-markdown': 'l1.2.6b'} and package B requires {'michelf/php-markdown': 'l1.2.6f'} there will be a conflict since Composer will only be able to install one version and two exact versions of PHP Markdown will have been specified.

On the other hand, if you tagged v1.2.6.2 instead of l1.2.6b someone would be able to require something like {'michelf/php-markdown': '1.2.*'}. If an application requires package A and package B, and package A requires {'michelf/php-markdown': '1.2.*'} and package B requires {'michelf/php-markdown': '~1.2.6'}, Composer will not se a conflict since non-exact versions are specified. (In this particular case, Composer would be free to select the newest 1.2 release greater than or equal to 1.2.6.)

I'd like to do whatever I can to help ensure that PHP Markdown can be installed using semantic versioning. If people start targeting specific versions of PHP Markdown things will get nasty pretty quickly and it will probably be doing more harm than good. Let me know what I can do to help things move in that direction. :)

@mauris
Copy link

mauris commented Jan 7, 2013

My simplest suggestion: fork into a new repo and clean everything up. Build up a neat repository.

@michelf
Copy link
Owner

michelf commented Jan 7, 2013

@thephpdeveloper I'd prefer keeping things in the same repository so they share the same issue tracking system and so I can merge more easily from one branch to the other. I intend to keep the two other branches alive for a little while.

@simensen One thing I might do is simply have no prefix for the lib branch. Don't worry about the letter suffix, they're going away (at lest for the lib branch).

About semantic versioning: the rules are pretty hard to apply when it comes to PHP Markdown. Any fix or change to the parser, even just adding a new feature, is likely to break things for somebody somewhere, because the new syntax doesn't build on something that was previously "illegal" (there is no such thing as illegal input in Markdown). So from a syntax perspective, every change should be a major version, but that doesn't make much sense. I'm going to have to figure out the rules, because that semver document doesn't apply very well here. (If you want to discuss this further, I suggest we open a new issue for this.)

@mauris
Copy link

mauris commented Jan 7, 2013

If your changes frequently breaks usage, it means that your public API is probably not well planned.

A well planned public API will minimize the BC break because whatever we have to change would probably be internal. Use the access modifier wisely.

@simensen
Copy link
Contributor

simensen commented Jan 7, 2013

@thephpdeveloper I think that the point @michelf is trying to make is that the actual parsing changes quite a bit but that the published API itself is stable.

Take fenced code blocks for example. Adding support for fenced code blocks will not change the published API in any way but documents that previously resulted in specifically formatted HTML may now result in different (and potentially unexpected/broken) HTML.

@michelf I think I see your reasoning though I think that it still makes sense to follow semantic versioning to some extent. Per your request I'll open another issue to discuss that though I don't have a lot of time to do that right now so it might be a day or so before I get around to it.

@mauris
Copy link

mauris commented Jan 7, 2013

@simensen I know. just simply take a few minor version changes to stabilize the published API. That's why we have beta.

@michelf
Copy link
Owner

michelf commented Jan 13, 2013

Done. The lib branch now has a compser.json file. But there's still a big but:

Unfortunately it seems that Packagist wants a composer.json file to be in the master branch, which completely breaks my plans about having a smooth transition. I'll have to think about this further. Currently, it seems to me like the only option would be to have a completely different repository, but that'd make it a pain to keep the three variants synchronized through git merges (and easy to mix things up, having two master branches).

(For reference, here's the exact error message that Packagist gives me: "No composer.json was found in the master branch.")

I'm leaving this issue open until this is resolved, somehow. Perhaps it's time I bring this up on the Packagist mailing list...

@baldurrensch
Copy link

What about putting just a very minimal composer.json file in the master branch? Like just author, package name, or so. I dunno if that will work, but it may be worth a try.

@michelf
Copy link
Owner

michelf commented Jan 13, 2013

Never mind. Setting the default branch to lib in Github's repository settings allowed Packagist to correctly fetch the lib branch. Done. :-)

https://packagist.org/packages/michelf/php-markdown

@michelf michelf closed this as completed Jan 13, 2013
@simensen
Copy link
Contributor

@michelf wow! What excellent news! Thanks for your hard work on this over all the years and for doing web more work to get it into the Composer ecosystem!

@mauris
Copy link

mauris commented Jan 13, 2013

@simensen indeed! thank you @michelf!

@joseym
Copy link
Author

joseym commented Jan 16, 2013

fantastic! thanks @michelf!

@sandeepshetty
Copy link

Awesome! Thanks @michelf.

@ghost
Copy link

ghost commented Jan 18, 2013

Please can you change the namespace to Michelf starting with a capital letter.

@michelf
Copy link
Owner

michelf commented Jan 18, 2013

@Drak What's the problem with the current namespace name?

@simensen
Copy link
Contributor

I think this is a personal preference. I brought this issue up on FIG mailing list awhile back and my takeaway from it was exactly what I brought to the list: neither PSR-0 nor PSR-1 address this and that most people seem to think it does not matter which way you roll. Sure, a healthy chunk of people are doing Vendor\Case\Stuff but some others do vendor\case\stuff. I think that it is fine as long as the project itself is consistent with itself.

For what it is worth, dflydev/markdown was one of the last projects I did with a lowercase dflydev vendor. I changed mostly because something like 95% of the packages I'm using used another casing so I figured I might as well get used to it. I did not do it because I thought it was "the right thing to do."

I still see other projects doing this from time to time. For example, webignition/internet-media-type does this.

@ghost
Copy link

ghost commented Jan 18, 2013

The reason some existing projects are not following things is because they started long before these issues were discussed. Class names must start with a capital letter, and while you can argue it's not 100% clear about the namespace, the intention of the PSRs were written by example, to use StudlyCaps specifically in the form mentioned. Projects that started before the PSRs continue to use their legacy. The PHP-FIG project also has it's own code examples to go by to see my point, look at PSR-3 https://github.com/php-fig/log where the namespace is as quoted.

@michelf
Copy link
Owner

michelf commented Jan 21, 2013

Done. The namespace is now Michelf (capitalized). I also added a Readme.php file which is some sort of sample code that parses the Readme.md file.

@ghost
Copy link

ghost commented Jan 21, 2013

👍

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

7 participants