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

Update README with the help command #339

Closed
wants to merge 1 commit into from
Closed

Update README with the help command #339

wants to merge 1 commit into from

Conversation

villfa
Copy link
Contributor

@villfa villfa commented Dec 22, 2021

I think it's a good addition to mention the help command.

In my case I would have saved me some time.
I couldn't find the list of the commands in the documentation.
Since I'm used of the Symfony commands I tried phive list (the command exists but doesn't return the list of the command) and phive --help without success.

Thanks

@theseer
Copy link
Member

theseer commented Dec 27, 2021

While I agree with the notion of having the help be more prominently shown, I fail to understand the underlying problem?

In contrast to your statement, phive --help works like char. As does phive without anything specified and, also logical to me at least, phive help.

I'll enhance the readme to showcase the help output but I'm afraid your simple line doesn't cut it. I'll thus not merge this.

@theseer theseer closed this Dec 27, 2021
theseer added a commit that referenced this pull request Dec 27, 2021
As suggested by #339
@villfa villfa deleted the readme branch December 27, 2021 13:55
@villfa
Copy link
Contributor Author

villfa commented Dec 27, 2021

Thanks for updating the readme. It's far better than what I proposed.

I fail to understand the underlying problem?

If you're interested I can give you more context.

The build of another project failed because Phive couldn't find the requested version of a tool (same issue than #274).
I wanted to try to fix it (and I still plan to do it, but I'm not sure when because of the holidays) so I've forked and cloned Phive before running composer install.
After that I looked for a way to run the tests and found the answer here: #323 (btw I think the readme should be updated to add more info about how to contribute).
So I ran ./phive install and it failed because it couldn't find the right version of psalm (again, same issue than #274).
My first reaction was to run ./phive install --help (because that's what I would do with others tools, like Composer for instance) and got this message:

[ERROR]    Error while processing arguments to command 'install':
[ERROR]    Unknown option: help

(perhaps it would be better to make the help option available for every command, WDYT?)

After that I tried ./phive install psalm but I took me time to understand the phive.xml file (and the constraints inside) was ignored when the command is run that way.
There is probably something to improve here too.

I won't bother you with the details but I also tried to uninstall the tools in order to run the installation again but I couldn't find the right way to do it.

@theseer
Copy link
Member

theseer commented Dec 28, 2021

Thanks for updating the readme. It's far better than what I proposed.

Thanks for pointing it out to begin with.

I fail to understand the underlying problem?

If you're interested I can give you more context.

Sure, otherwise I wouldn't have asked ;-)

The build of another project failed because Phive couldn't find the requested version of a tool (same issue than #274).

Sorry about that. It's a really stupid issue that shouldn't have been there to begin with as we should have kept the resolved result within the phive.xml file. I'll see if i can fix that over the next couple of days - but no promises.

I wanted to try to fix it (and I still plan to do it, but I'm not sure when because of the holidays) so I've forked and cloned Phive before running composer install. After that I looked for a way to run the tests and found the answer here: #323 (btw I think the readme should be updated to add more info about how to contribute).

Certainly true. We're notoriously bad with documentation I guess...

So I ran ./phive install and it failed because it couldn't find the right version of psalm (again, same issue than #274). My first reaction was to run ./phive install --help (because that's what I would do with others tools, like Composer for instance) and got this message:

[ERROR]    Error while processing arguments to command 'install':
[ERROR]    Unknown option: help

(perhaps it would be better to make the help option available for every command, WDYT?)

Yes, see #253

After that I tried ./phive install psalm but I took me time to understand the phive.xml file (and the constraints inside) was ignored when the command is run that way. There is probably something to improve here too.

I really wonder what the expected behavior would be here actually. Right now we deliberately choose to ignore any given constraints from the phive.xml / .phive/phars.xml files. Not sure if that's the best way but it made sense to me at the time ;)

I won't bother you with the details but I also tried to uninstall the tools in order to run the installation again but I couldn't find the right way to do it.

phive remove foo ?

@villfa
Copy link
Contributor Author

villfa commented Dec 28, 2021

I won't bother you with the details but I also tried to uninstall the tools in order to run the installation again but I couldn't find the right way to do it.

phive remove foo ?

phive remove modifies phive.xml that's why I thought there could be another way to uninstall a tool.
I ended up doing this:

phive remove foo
phive purge
git checkout -- phive.xml

@theseer
Copy link
Member

theseer commented Dec 28, 2021

You lost me ;)

Why would the fact it is removed from the phive.xml be a problem when you want to re-install with a different constraint anyway?

Are you telling me, we need to add a --temporary option to the remove command like it exists for install?

@villfa
Copy link
Contributor Author

villfa commented Dec 28, 2021

When the installation of the tools required by Phive failed, I wasn't sure why it happened so I wanted to run the installation again to debug it.
I was looking for a command like uninstall or clean, but your proposal would have worked too.

@theseer
Copy link
Member

theseer commented Dec 28, 2021

Ah! Maybe we indeed should add a "clean" command... 🤔

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