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

docs: add instructions for testing a PR by installing a downloaded binary in the $PATH #5451

Merged
merged 8 commits into from Nov 22, 2023
Merged

Conversation

gitressa
Copy link
Contributor

@gitressa gitressa commented Oct 24, 2023

The Issue

It is documented how to test a PR in macOS with Homebrew, but not by installing a downloaded binary in the $PATH.

How This PR Solves The Issue

Documents how to test a PR by installing a downloaded binary in the $PATH, to be used for example in Linux and MacOS.

Manual Testing Instructions

Automated Testing Overview

Related Issue Link(s)

Release/Deployment Notes

@gitressa gitressa requested a review from a team as a code owner October 24, 2023 10:02
@gitressa gitressa changed the title Add Linux instructions for testing a PR docs: Add Linux instructions for testing a PR Oct 24, 2023
@gitressa gitressa changed the title docs: Add Linux instructions for testing a PR docs: add Linux instructions for testing a PR Oct 24, 2023
Copy link
Member

@stasadev stasadev left a comment

Choose a reason for hiding this comment

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

Thank you!

docs/content/developers/building-contributing.md Outdated Show resolved Hide resolved
docs/content/developers/building-contributing.md Outdated Show resolved Hide resolved
docs/content/developers/building-contributing.md Outdated Show resolved Hide resolved
docs/content/developers/building-contributing.md Outdated Show resolved Hide resolved
@gitressa gitressa mentioned this pull request Oct 24, 2023
1 task
@gitressa
Copy link
Contributor Author

Thanks for the comments @stasadev. Before addressing them, I am having an odd situation, where my DDEV 1.22.3 without a DDEV in .local/bin gives me the ddev dbeaver feature ... Do you have any idea how to clean Docker images, and start fresh? I ran ddev poweroff && docker system prune -a --volumes but maybe I need to clear some more Docker or DDEV caches as well?

@stasadev
Copy link
Member

I am having an odd situation, where my DDEV 1.22.3 without a DDEV in .local/bin gives me the ddev dbeaver feature

It is because the file is here ~/.ddev/commands/host/dbeaver.
You can simply remove it.

If you want to have a complete fresh DDEV, you can backup ~/.ddev/global_config.yaml (you need only project_info key section from there), remove ~/.ddev folder, and then run some ddev command (like ddev version), it will recreate ~/.ddev folder, and you can add back project_info to ~/.ddev/global_config.yaml.

Do you have any idea how to clean Docker images.

I usually do ddev poweroff && ddev delete images --all when I want to clean DDEV related stuff.

and

docker rm -f $(docker ps -aq)
docker rmi -f $(docker images -q)
docker builder prune

when I want to clean everything else.

Or as you did: docker system prune -a --volumes - this command cleans everything. I usually run this if I've messed something up and want to start over.

Copy link
Member

@rfay rfay left a comment

Choose a reason for hiding this comment

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

This really isn't about linux at all. It's about understanding how $PATH works, and putting the ddev binary in your PATH ahead of where the system-installed binary is.

I'd love to have you rethink this as "installing a downloaded binary in the $PATH" instead of "Linux instructions".

@gitressa gitressa changed the title docs: add Linux instructions for testing a PR docs: add instructions for testing a PR by installing a downloaded binary in the $PATH Oct 24, 2023
@rfay
Copy link
Member

rfay commented Oct 24, 2023

Another minor nuance is that this isn't just about testing PRs, it's the same with testing HEAD version by downloading the artifacts.

@rfay
Copy link
Member

rfay commented Oct 24, 2023

And... It is in fact hard for lots of people. So this is a very valuable thing to cover.

@tyler36
Copy link
Collaborator

tyler36 commented Oct 25, 2023

Using echo $PATH and which ddev are valuable commands for debugging.

@gitressa
Copy link
Contributor Author

Awesome explanation about Docker and the different DDEV parts @stasadev. I didn't get to comment about it before now, but thanks a lot for that, I really appreciate it.

Maybe the steps to test a PR (or the structure of DDEV itself and tools and add-ons?) needs to get a short description, since there are a few moving parts ...?

Maybe the help text could be expanded, to cover testing a PR for different situations and what to be aware of, such as testing a PR which changes code only inside DDEV (which I suppose would follow the latest binary) or testing a PR, which changes code in other places, such as the ~/.ddev folder?

@tyler36: Great suggestion, I have updated the text.

Copy link
Sponsor Collaborator

@mattstein mattstein left a comment

Choose a reason for hiding this comment

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

Left some nitpicky suggestions but otherwise looks good to me!

docs/content/developers/building-contributing.md Outdated Show resolved Hide resolved
docs/content/developers/building-contributing.md Outdated Show resolved Hide resolved
@gitressa
Copy link
Contributor Author

Perfect @mattstein, both were great suggestions, thanks!

@gitressa
Copy link
Contributor Author

In a follow up issue, perhaps that page can be re-visited about the use of the word "Binary"?

It doesn't mean anything to me.

So perhaps the page can start with stating something like this?

After each PR, an individually-compiled DDEV binary ("DDEV binary") is created containing the new code.

And after that, use "DDEV binary" on the page or whatever we agree on is a fitting word for it, and not just "binary"?

@gitressa
Copy link
Contributor Author

In a follow up issue, perhaps the page can be re-visited about the use of the word "Binary"?

It doesn't mean anything to me.

So perhaps the page can start with stating something like this?

After each PR, an individually-compiled DDEV binary ("DDEV binary") is created containing the new code.

And after that, use "DDEV binary" on the page or whatever we agree on is a fitting word for it, and not just "binary"?

Also, this could be tied in with @rfay's comment about DDEV’s latest-committed HEAD version:

Another minor nuance is that this isn't just about testing PRs, it's the same with testing HEAD version by downloading the artifacts.

I am not certain how to phrase it though ...

@rfay
Copy link
Member

rfay commented Oct 26, 2023

I'm interested in what wording to use, if we can do better. "Artifacts" is a pretty technical term which is not useful to people who aren't normally in a build environment. "Binary" is one of many words that imply a compiled artifact. In the PHP world there's aren't binaries (except php itself of course) because things are scripted. Looking forward to your thoughts about better words to use. Thoughts, @mattstein ?

@mattstein
Copy link
Sponsor Collaborator

Maybe executable instead?

@rfay
Copy link
Member

rfay commented Oct 26, 2023

I could live with "executable". It's kind of a traditional Windows-ish word, but maybe people would understand it better.

@mattstein
Copy link
Sponsor Collaborator

If that’s the predominant term in the GoLang docs, I don’t think Clippy crossover warrants great concern. 🤷‍♂️

Copy link
Member

@rfay rfay left a comment

Choose a reason for hiding this comment

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

Thanks for this contribution and initiative.

Overall, this is adding too much training for too little value. I think we have to (mostly) assume that people understand what $PATH is. We can give them a little guidance about the place to put it.

IMO ~/bin is a better place to put things than .local/bin, it's easier to work with and supported by most platforms' .bash_profile.

Overall the problem though is how much handholding we can do here.

Could we do a gist or something with the full handholding and keep this more concise?

Surely there's a good tutorial somewhere we can point people to, or a gist or something?

Or maybe we need a PATH tutorial on ddev.com?

@@ -15,6 +15,8 @@ Each [PR build](https://github.com/ddev/ddev/actions/workflows/pr-build.yml) cre

Download and unzip the appropriate binary and place it in your `$PATH`.

### Homebrew with macOS
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
### Homebrew with macOS
### Homebrew with macOS or Linux

@rfay rfay marked this pull request as draft November 2, 2023 22:07
@gitressa
Copy link
Contributor Author

gitressa commented Nov 3, 2023

Or maybe we need a PATH tutorial on ddev.com?

That's a great idea! We can add the text from this patch there, and link to it.

Also, ~/bin isn't in my path on Ubuntu ...

@rfay
Copy link
Member

rfay commented Nov 3, 2023

On Ubuntu and most systems, this is in /etc/skel/.profile, which is used by default by sh and bash:

# set PATH so it includes user's private bin if it exists
if [ -d "$HOME/bin" ] ; then
    PATH="$HOME/bin:$PATH"
fi

# set PATH so it includes user's private bin if it exists
if [ -d "$HOME/.local/bin" ] ; then
    PATH="$HOME/.local/bin:$PATH"
fi

That's most likely how your .local/bin gets into the PATH, and if ~/bin exists, it will be added.

You have to start a new bash session to see it.

@gitressa
Copy link
Contributor Author

gitressa commented Nov 3, 2023

I didn't know about /etc/skel/.profile @rfay, thanks for explaining. And you're right, it's there, and a copy of it in ~/.profile, and if I create ~/bin it does get picked up as part of the $PATH.

I prefer to keep my home folder as clean as possible of any program folders, which is why I use the hidden folder ~/.local/bin for this, to house Drush, Composer, and so on.

@rfay rfay marked this pull request as ready for review November 22, 2023 18:18
@rfay rfay merged commit 500dd1c into ddev:master Nov 22, 2023
12 checks passed
@gitressa
Copy link
Contributor Author

Perfect, thanks @mattstein, @stasadev, @tyler36, and @rfay.

Actually, I have since come to the conclusion that using ~/bin for DDEV-testing is the superior solution, since after you're done, you can just delete the folder, to clean up everything. And then ... might this be slightly better, since you then don't have to take care of the ZIP-file?

cd ddev ~/bin

Next, download the ZIP-file, unzip it and make it executable. Check with echo $PATH:

unzip ddev.zip && chmod +x ddev

Or maybe just add the the rm command like so unzip ddev.zip && rm ddev.zip in the current solution?

Also, both v1.22.3 and v1.22.5 are used, but that's a minor detail.

@rfay
Copy link
Member

rfay commented Nov 23, 2023

I always keep a ~/bin, and it's a fairly normal thing to do since the beginning of the Unix age.

Let's let this be for now. Maybe you'll want to return to it in a few months :)

@gitressa
Copy link
Contributor Author

Hah, yes, I may realize that as well :)

@gitressa gitressa deleted the patch-2 branch November 23, 2023 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants