Skip to content

Utilize composer's 'bin' property for global installs.#41

Open
jhedstrom wants to merge 1 commit into
joachim-n:masterfrom
jhedstrom:composer-bin
Open

Utilize composer's 'bin' property for global installs.#41
jhedstrom wants to merge 1 commit into
joachim-n:masterfrom
jhedstrom:composer-bin

Conversation

@jhedstrom

Copy link
Copy Markdown

This change makes use of composer's bin property so that when installing globally, or as part of another project, the binary is properly symlinked into the bin directory (typically vendor/bin).

I wonder if for backward compatibility if we should symlink the binary back into the root directory?

@jhedstrom jhedstrom force-pushed the composer-bin branch 2 times, most recently from 9e117de to a64a668 Compare January 29, 2019 18:38
Comment thread composer.json Outdated
}
}
},
"bin": ["bin/dorgflow"]

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we just leave the dorgflow file where it is, and give a different value to composer in the 'bin' property? Looking at how the bin/ part is given here, it doesn't look like the bin/ folder is magic in any way.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, good idea! I'll make that change--I think the changes to the binary's require statements will still be needed as this gets symlinked into composer's bin directory.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fine -- the DIR is generally better practice anyway, as it makes it explicit that it's the current directory. Without it, the code would break if it were to be moved to go after a chdir().

@jhedstrom

Copy link
Copy Markdown
Author

I think the necessary changes have been made here. Anything left to do?

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

Successfully merging this pull request may close these issues.

2 participants