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

Make code contributions easier with Gitpod #2783

Merged
merged 8 commits into from
Jan 21, 2019

Conversation

jankeromnes
Copy link
Contributor

@jankeromnes jankeromnes commented Jan 16, 2019

Hi @paulmelnikow, I've finally managed to make it work. 🎉 Thanks a lot for your help! 😄

As a reminder, Gitpod is a service that provides free readily configured workspaces in the cloud, complete with a lightweight IDE, a terminal, and a web preview.

What this pull request does is to pre-configure all Shields workspaces with ports 8080 and 3000 exposed, and on start-up they will automatically run npm install, npm run build and finally start the server with node server 8080 0.0.0.0.

A few caveats:

  • Opening the web app in a Browser tab works better than the Preview pane, because the latter has a dark background.
  • I haven't pre-installed ImageMagick, but I can still add it just in case

If you'd like to try out this Gitpod configuration, here is a snapshot:

GitHub issues by-label

And here is what it looks like:

screenshot 2019-01-16 at 18 59 26

Please have a look when you have time. 🙂

@shields-ci
Copy link

shields-ci commented Jan 16, 2019

Messages
📖 ✨ Thanks for your contribution to Shields, @jankeromnes!
📖

Thanks for contributing to our documentation. We ❤️ our documentarians!

Generated by 🚫 dangerJS against ef47ffe

@paulmelnikow
Copy link
Member

This is neat!

How do I get the browser window to come up on port 8080 once I close that popup?

Some quick feedback so far: this is set up to work on the backend but not the frontend. It builds the frontend, and then serves it through the backend server. I think that is totally fine and probably where Gitpod would come in more valuable since it makes it easy for a Ruby developer to build an endpoint really quickly. However, if that's the case there's no need to mention port 3000 in the config; we're not using it.

The best way to develop services is by running service tests. Could we set up a way to trigger a service test run?

@jankeromnes
Copy link
Contributor Author

jankeromnes commented Jan 16, 2019

This is neat!

Many thanks!

How do I get the browser window to come up on port 8080 once I close that popup?

Normally the popup should come back whenever you start something on port 8080, allowing you to either open the Preview pane or in a new Browser tab.

Alternatively, the preview URL is simply your Gitpod workspace URL but with a 8080- prefix on the domain, so you can also open that in a new tab.

Some quick feedback so far: this is set up to work on the backend but not the frontend. It builds the frontend, and then serves it through the backend server. I think that is totally fine and probably where Gitpod would come in more valuable since it makes it easy for a Ruby developer to build an endpoint really quickly. However, if that's the case there's no need to mention port 3000 in the config; we're not using it.

Thank you for explaining this. Out of curiosity, what would be a good setup for working on the frontend? And is there a setup that is great for both backend and frontend development?

Could we set up a way to trigger a service test run?

Sure, good idea! I can add a command to the .gitpod.yml file. Should we do something like this?

  - command: npm test && node server 8080 0.0.0.0

(Note: This will run npm test && node server 8080 0.0.0.0 automatically every time you open a Shields workspace.)

And is npm test the best command to run here?

@paulmelnikow
Copy link
Member

Normally the popup should come back whenever you start something on port 8080, allowing you to either open the Preview pane or in a new Browser tab.

Ah, okay. How do I restart the server?

Alternatively, the preview URL is simply your Gitpod workspace URL but with a 8080- prefix on the domain, so you can also open that in a new tab.

👍

It would be good to add someplace in the UI where that can be triggered!

Should we do something like this?

  - command: npm test && node server 8080 0.0.0.0

(Note: This will run npm test && node server 8080 0.0.0.0 automatically every time you open a Shields workspace.)

I don't think that's really what we want.

Starting the server makes sense by default. It would then be good if the developer could make some changes and then trigger the tests. The most common task would be changing the services and running a subset of the service tests, which looks like this:

It looks like this: https://github.com/badges/shields/blob/master/doc/service-tests.md#3-running-the-tests

Also running the unit tests could be helpful. npm run test:server is the one I run the most frequently.

Also the linter and the code formatter; npm run lint and npm run prettier. Do you guys integrate Prettier on save, by the way?

@paulmelnikow paulmelnikow added the developer-experience Dev tooling, test framework, and CI label Jan 16, 2019
@jankeromnes
Copy link
Contributor Author

Ah, okay. How do I restart the server?

In the Terminal, you can use Ctrl+C to stop the server, and run node server 8080 0.0.0.0 again to restart it.

It would be good to add someplace in the UI where that can be triggered!

You're right! I don't think there is currently a view of "currently active ports and how to preview them". I'll open an issue for that in Theia IDE.

Thank you for all these great ideas. I agree that we should find a good way to make it easy for developers to discover and run these commands in particular:

npm run test:services -- --only=<SERVICE>
npm run test:server
npm run lint
npm run prettier

Do you guys integrate Prettier on save, by the way?

Regular Git commit & push hooks are supported (I've noticed husky and other things running when I was hacking on Shields in Gitpod), but I'm not sure if there is a way to configure commands to be run on save. Does Prettier run fast enough to keep the "saving" action reasonably short?

@paulmelnikow
Copy link
Member

You're right! I don't think there is currently a view of "currently active ports and how to preview them". I'll open an issue for that in Theia IDE.

👍

Regular Git commit & push hooks are supported (I've noticed husky and other things running when I was hacking on Shields in Gitpod), but I'm not sure if there is a way to configure commands to be run on save. Does Prettier run fast enough to keep the "saving" action reasonably short?

Yes! You can't run npm run prettier that way because it runs on all the files and is therefore far too slow. However Prettier is designed to be integrated into editors and triggered on save: https://prettier.io/docs/en/editors.html Maybe that's another feature request for theia-ide! FWIW Prettier is an incredibly nice experience and I'm sure it would be used by lots of other projects.

npm run test:services -- --only=<SERVICE>
npm run test:server
npm run lint

Yea, agreed! It would be good to have a list of registered programs that can be triggered. If it came down to it, I think we could work around the unusual case of npm run test:services -- --only=<SERVICE> by inserting an .only() in the code.

@jankeromnes
Copy link
Contributor Author

jankeromnes commented Jan 16, 2019

In general, I think Theia and Gitpod need a way to expose useful project-specific workflows, e.g. via something like "Tasks" (in the Terminal top menu, there is a thing called Run Task..., but it's quite hard to find, and it doesn't seem to have any tasks available).

What I'd find really cool would be to have a Shields top menu (e.g. next to Terminal) offering these useful project workflows. And I think it could be easily configurable in .gitpod.yml. I'd love to make that happen.

EDIT: It's a bit late for me today, but I've made a note to file these issues and get the ball rolling tomorrow. 😄

@jankeromnes
Copy link
Contributor Author

Update: I've filed the following Gitpod issues:

This Theia issue was already open:

@paulmelnikow
Copy link
Member

Nice! I added a comment on one of those.

gitpod-io/gitpod#246 is about the CLI; is it a separate matter to surface it in the UI through a menu or a button?

@jankeromnes
Copy link
Contributor Author

jankeromnes commented Jan 17, 2019

Thanks! And indeed, it's true that gitpod-io/gitpod#246 doesn't help listing all currently active ports. Except if we implement the gp url --all I suggested -- do you think that would be valuable?

Regarding showing running ports in the UI, isn't that already the case? E.g. if you start a web server on any port, Theia will open a pop-up to let you know, and offer to Expose the port, or to open it in a Preview pane or Browser tab. Are you suggesting an additional view somewhere in the UI that recaps all currently available previews?

@paulmelnikow
Copy link
Member

Regarding showing running ports in the UI, isn't that already the case? E.g. if you start a web server on any port, Theia will open a pop-up to let you know, and offer to Expose the port, or to open it in a Preview pane or Browser tab. Are you suggesting an additional view somewhere in the UI that recaps all currently available previews?

What happened to me is that the popup appeared first and I did not initially click on it. Then I closed the window and reopened it, and the popup did not re-appear (as I understand it, this was because the service had already been running). At that point I had no idea how to get it back.

One idea would be to re-surface the open ports when the window is reopened; another is to add an option to the menus to open the preview or browser tab.

@paulmelnikow
Copy link
Member

It seems like this may be an ongoing process 😁

What do you think about reviewing and merging this, and continuing the discussion / work of refining this in an issue?

Related: I've been thinking about hosting a distributed hack day / Open Source Friday-type event focused on bringing in new contributors to implement our backlog of badge requests: https://github.com/badges/shields/issues?q=is%3Aissue+is%3Aopen+label%3Aservice-badge

As contributors come from many different programming communities, not necessarily Node / JS, easing the barrier to contribution could be a big help for helping them land changes like that.

I think there is a bit of work to do to make a functional enough workflow for the kind of TDD we do, though if you're down to keep plugging away at this I think we could get there.

I'd be down to partner with Gitpod on that event if that's something you might be interested in!

@jankeromnes
Copy link
Contributor Author

Thanks a lot for your feedback. I also sometimes see "Open Port" notifications getting dismissed too soon.

One idea would be to re-surface the open ports when the window is reopened; another is to add an option to the menus to open the preview or browser tab.

Both are excellent ideas that we should probably implement. 👍 I've filed gitpod-io/gitpod#248 for that.

It seems like this may be an ongoing process 😁

Haha, true, I guess the experience can always be incrementally improved!

What do you think about reviewing and merging this, and continuing the discussion / work of refining this in an issue?

Works for me! I'll rebase this pull request and I'm happy to address any review comments or suggestions.

Related: I've been thinking about hosting a distributed hack day / Open Source Friday-type event focused on bringing in new contributors to implement our backlog of badge requests: https://github.com/badges/shields/issues?q=is%3Aissue+is%3Aopen+label%3Aservice-badge

Indeed, that's quite good material for a hackathon already. Is the process always similar to fix this type of issue? (If so, you could probably explain it at the beginning of the event, and then let everyone loose on the code.)

As contributors come from many different programming communities, not necessarily Node / JS, easing the barrier to contribution could be a big help for helping them land changes like that.

I think there is a bit of work to do to make a functional enough workflow for the kind of TDD we do, though if you're down to keep plugging away at this I think we could get there.

I'd be down to partner with Gitpod on that event if that's something you might be interested in!

This sounds amazing! I agree that participants already have plenty enough on their plate learning how the project works, and finding their way around issues and code files, especially if they're not familiar with the language or framework in use. So any "boring" setup / installation / registration / OS-specific issues should definitely be alleviated as much as possible.

I'm definitely down to keep plugging away at this because Shields is one of the coolest community projects I know (so I'm personally happy to help out!) and we also want to make Gitpod as friendly / intuitive / supportive as we can for people who are new to a project or to programming in general.

Perhaps we could chat by email or video to work out the specifics of this partnership 😄 Many thanks for bringing up this really cool idea!

@paulmelnikow
Copy link
Member

paulmelnikow commented Jan 17, 2019

Indeed, that's quite good material for a hackathon already. Is the process always similar to fix this type of issue? (If so, you could probably explain it at the beginning of the event, and then let everyone loose on the code.)

Yea, we have a good tutorial about this process: https://github.com/badges/shields/blob/master/doc/TUTORIAL.md

Perhaps we could chat by email or video to work out the specifics of this partnership 😄 Many thanks for bringing up this really cool idea!

Thanks for the kind words! and that sounds great! Shoot an email to the address on my profile and we can get something set up.

I'll rebase this pull request and I'm happy to address any review comments or suggestions.

Merging in master is fine / preferred; we use a squash workflow.

.gitpod.yml Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
@paulmelnikow
Copy link
Member

I left a few comments.

It would also be good to open a new issue for Gitpod improvement, troubleshooting, and support during the "experimental" period, and linking to that from the contributing guidelines.

@jankeromnes
Copy link
Contributor Author

jankeromnes commented Jan 17, 2019

Yea, we have a good tutorial about this process: https://github.com/badges/shields/blob/master/doc/TUTORIAL.md

Ah right, I hadn't recognized the service-badge label. I know this tutorial from trying to add a Gitpod badge 😊 (but given that it's static, it can already be created with a simple URL -- kudos to Shields!)

EDIT: The "Lines of code (LOC) badge" one looks particularly exciting!

Thanks for the kind words! and that sounds great! Shoot an email to the address on my profile and we can get something set up.

Email sent!

I left a few comments.

It would also be good to open a new issue for Gitpod improvement, troubleshooting, and support during the "experimental" period, and linking to that from the contributing guidelines.

Thanks! And good idea, I'll do that. Would you prefer a new issue or should we continue in #2772?

@paulmelnikow
Copy link
Member

Ah, #2772 is good 👍

@jankeromnes
Copy link
Contributor Author

jankeromnes commented Jan 21, 2019

Merging in master is fine / preferred; we use a squash workflow.

Oops, sorry, I seem to have missed this line...

I've rebased the pull request to remove the Gitpod SVG logo; the port 3000; and to replace the Gitpod Shields badge with an SVG button.

@paulmelnikow could you please have another look when you have time?

@paulmelnikow
Copy link
Member

Could you add the support link and reset the lockfile?

@jankeromnes
Copy link
Contributor Author

Could you add the support link and reset the lockfile?

Ah, right, I'll add a link to #2772. (Thanks for having already improved my Gitpod intro paragraph!)

@jankeromnes
Copy link
Contributor Author

Thanks a lot for the fix-ups and the review!

How do you reset the lockfile by keeping these "optional": true fields?

@paulmelnikow
Copy link
Member

How do you reset the lockfile by keeping these "optional": true fields?

The churn on optional has to do with a difference in the way the npm CLI handles things on Linux and on Mac. I couldn't immediately put my hands on the link but maybe one of the other maintainers can.

When dependencies have changed we live with the churn; otherwise you can run git checkout master package-lock.json.

@paulmelnikow paulmelnikow merged commit 3695689 into badges:master Jan 21, 2019
@shields-deployment
Copy link

This pull request was merged to master branch. This change is now waiting for deployment, which will usually happen within a few days. Stay tuned by joining our #ops channel on Discord!

After deployment, changes are copied to gh-pages branch:

@paulmelnikow
Copy link
Member

Looking forward to chatting more about this soon!

@jankeromnes
Copy link
Contributor Author

Many thanks @paulmelnikow! I'm also really looking forward to chat more about this, and to continuously make development easier for Shields. 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
developer-experience Dev tooling, test framework, and CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants