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

Add a devcontainer configuration #163

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

Conversation

oplik0
Copy link

@oplik0 oplik0 commented Jun 18, 2022

This is a fairly useful tool for mainly GitHub Codespaces users (though it should also work with Docker Desktop and self-hosted remote hosts.

This specific configuration provides a NodeBB instance running in the background (using grunt to rebuild on changes) with the currently developed plugin linked to it and activated already.

Some issues:

  1. built time is fairly long - in my testing on Codespaces it takes ~1 minute and 30 seconds to get the container booted for VSCode (most of which seems to be spent on fetching and extracting Docker images) and then another ~3 minutes to get to running NodeBB instance. Here most is spent on the initial yarn install and setup, the actual build with bundling takes <10 seconds.
    potential solutions:
    • build.cacheFrom - this would probably require publishing an image to use as cache, and to really help require moving NodeBB setup into docker so it is actually cached.
    • Codespaces Prebuilds - this is something that has to be set up in the repo that's actually used for development (so here NodeBB has no control), but should work fine since the NodeBB installation is in the hook that is also part of prebuild process. EDIT: tried prebuilds and it works great, as the longest step became installing the plugin dependencies. I moved it to updateContentCommand which will also run when prebuilding. I think it should be possible to get ~30 second startup from a prebuilt codespace.
  2. The only sensible way to restart NodeBB is to make changes in the plugin or NodeBB itself that grunt will pick up - this is just a sometimes slightly annoying issue with using grunt; the restart in admin UI just doesn't seem to work. I think it's fairly minor and I haven't looked up into how one would go about fixing it in grunt.
  3. config.json has hardcoded localhost URL which may become a slight issue when for example sharing NodeBB port online (with Codespaces specifically) or just mapping it to a different port on host. Most things still work in these cases though and it's simple to change in the container (though here a better way to restart NodeBB would be useful), but it can break some things (for example SSO plugins). This seems like a fairly rare problem and I encountered it only once (with an SSO plugin as mentioned).
  4. To understand what's where and how to use it one probably needs to read the devcontainer files.
    This is IMO currently the biggest issue and I'm just not entirely sure how should I resolve it - I don't think this deserves a section in main README, but perhaps a separate README in the .devcontainer folder would be fine?
    There isn't much to explain, it mostly just works, but I think it might be a good idea to have a single place with basic information like credentials used or path to NodeBB.

I have tested it with Codespaces and some time ago also in Docker Desktop and it worked fine then (and my changes since shouldn't have affected anything there). I'm also using a very similar setup for a few plugins already. I find it to be quite nice dx to have a fresh instance for each plugin ready without having to do any further setup :)

I marked it as draft due to the documentation issue - please let me know where I should put something (or whether it's needed).

@oplik0 oplik0 marked this pull request as ready for review June 21, 2022 17:40
@oplik0
Copy link
Author

oplik0 commented Jun 21, 2022

I have added some basic documentation under .devcontainer/README.md

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.

1 participant