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

Display current git branch in the terminal prompt (PS1) #447

Merged
merged 2 commits into from
Jul 20, 2021

Conversation

Shulammite-Aso
Copy link
Contributor

@gitpod-io
Copy link

gitpod-io bot commented Jul 14, 2021

@Shulammite-Aso
Copy link
Contributor Author

@jankeromnes, please take a look.

@jankeromnes jankeromnes self-requested a review July 15, 2021 10:49
@jankeromnes
Copy link
Contributor

Super-cool! Many thanks @Shulammite-Aso for taking on & fixing this issue 💯 x 🙏

Also, sorry for the delay in reviewing this! I'll take a look now, and I'll also make CircleCI build your PR so that we get images we can test on actual repositories. 😊

@Shulammite-Aso
Copy link
Contributor Author

Alrigth @jankeromnes, can't wait! 😃

Copy link
Contributor

@jankeromnes jankeromnes left a comment

Choose a reason for hiding this comment

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

Thanks again for this great work @Shulammite-Aso 😁

I believe it's almost perfect as is, but noticed a very small issue, that basically just requires adding a few \ to the correct places.

Please simply add the missing \ where expected (see the code suggestion below), and then I'll trigger a new CircleCI build and we'll be able to test the updated fix in Gitpod.

I've also added some more thoughts & details (sorry for being so verbose 😅) but feel free to skip them if you like. (Or, if you have any question / if anything seems unclear, please let me know and I'll be happy to help!)

base/Dockerfile Outdated Show resolved Hide resolved
@Shulammite-Aso
Copy link
Contributor Author

Okay @jankeromnes. Taking a moment to read, understand, then make the necessary changes. Will let you know if I have a question.

@Shulammite-Aso
Copy link
Contributor Author

@jankeromnes, I get this now and trying to make the changes, but I just saw that I have reached my monthly workspace hour limit, sadly. Now I don't know if there's any other way around this? I've been playing around using the workspaces for the past weeks. It almost slipped my mind that I'm on a very limited plan. 🤦‍♀️

@jankeromnes
Copy link
Contributor

jankeromnes commented Jul 19, 2021

trying to make the changes, but I just saw that I have reached my monthly workspace hour limit

Oh no 🙈 sorry for the disappointing "surprise" there! Given that you're contributing to Gitpod, I've just granted you 20 bonus hours, so that you can still use Gitpod when your "regular" monthly hours run out. (Please let me know if this ever blocks you again!)

@Shulammite-Aso
Copy link
Contributor Author

commited!

Given that you're contributing to Gitpod, I've just granted you 20 bonus hours,

Thanks for this. Will start keeping my limit in mind.

@jankeromnes
Copy link
Contributor

jankeromnes commented Jul 20, 2021

Alright, many thanks for the fix! This now looks perfect. ✨

I've also made CircleCI build this Pull Request, which resulted in the Docker image gitpod/workspace-base:branch-pr-447 getting published to Docker Hub. Let's hook this up with a repository, and briefly test it in Gitpod, to confirm that everything works as intended.

Updates:

jankeromnes added a commit to jankeromnes/test-gitpod-prompt that referenced this pull request Jul 20, 2021
Copy link
Contributor

@jankeromnes jankeromnes left a comment

Choose a reason for hiding this comment

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

Alright, this works like a charm! 🎉 🎆 Thus I'm approving this Pull Request, and it can be merged soon.

Screenshot 2021-07-20 at 09 35 49

Thanks again for implementing this!


I've just noticed one tiny problem, but it seems completely unrelated to this code change (probably a random new bug in our code): Every new IDE Terminal now shows this error as the first line:

ls: cannot access '/home/gitpod/.bashrc.d/*': No such file or directory

I'll investigate this a bit more, to see if I can find a fix for it, but otherwise I think we should merge your PR quite soon. 😊

@Shulammite-Aso
Copy link
Contributor Author

Alright, this works like a charm! 🎉 🎆 Thus I'm approving this Pull Request, and it can be merged soon.

nice to see!! 💃 🎉 🎉

@Shulammite-Aso
Copy link
Contributor Author

Thanks for letting me contribute @jankeromnes!! 😁 😁

@jankeromnes
Copy link
Contributor

ls: cannot access '/home/gitpod/.bashrc.d/*': No such file or directory

I'll investigate this a bit more, to see if I can find a fix for it, but otherwise I think we should merge your PR quite soon. 😊

Ah, I think I understand the problem now -- to test this Pull Request, I've used the gitpod/workspace-base image instead of the more complete gitpod/workspace-full. It would seem that this line logs an error when the $HOME/.bashrc.d directory is empty (while instead it should not log an error and just do nothing):

(echo; echo "for i in \$(ls \$HOME/.bashrc.d/*); do source \$i; done"; echo) >> /home/gitpod/.bashrc

In other words, this bug existed before, and has indeed nothing to do with this Pull Request -- so I'll merge this now, and fix the bug in a separate Pull Request. 😁 🙌

Thanks for letting me contribute @jankeromnes!! 😁 😁

You're quite welcome! 😁 And all the thanks go to you, for doing all the work 🙏

@jankeromnes jankeromnes merged commit 2e5340e into gitpod-io:master Jul 20, 2021
@Shulammite-Aso
Copy link
Contributor Author

You're quite welcome! 😁 And all the thanks go to you, for doing all the work 🙏

🎉🎉🎉

@jankeromnes I'm also willing to make another contribution! I know I should probably not take up another good-first-issue, as it will no longer be my first. Unless it's absolutely okay to do that.:smile: Otherwise I can make changes to a react/typescript and nodejs/typescript codebase. I'm yet to comfortably pick up Golang, hopefully, I do so soon.

@ArthurSens
Copy link

Hey @jankeromnes, is this deployed alongside Gitpod when we update production? I'm not sure if I'm supposed to move this to "Done" in the groundwork project 🤔

@ArthurSens
Copy link

Friendly ping for the question above /\

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Display the current Git branch in the Terminal prompt (PS1)
3 participants