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

Improve TL;DR documentation for starters #215

Closed
wants to merge 12 commits into from

Conversation

microstudi
Copy link
Contributor

@microstudi microstudi commented Nov 12, 2021

I've found quite impossible to install this and make it work.

This PR is an attempt to document a little bit better the steps needed in order for this to work properly with a Decidim installation.

  • Fixes a Sidekiq configuration with servers other than localhost (note that this could be improved to allow passwords as well)
  • Improves README instructions for easy setup
  • Adds a "production" docker-compose.yml for easy deployment

@microstudi microstudi changed the title Improve TL;DR documentation for startes Improve TL;DR documentation for starters Nov 12, 2021
andreslucena
andreslucena previously approved these changes Nov 25, 2021
Copy link
Member

@andreslucena andreslucena left a comment

Choose a reason for hiding this comment

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

I'd love to have this a couple of months ago 😅
Awesome work!! This is much better than what we have at the moment, for sure.
I have some small improvements

bulletin_board/server/README.md Outdated Show resolved Hide resolved
bulletin_board/server/README.md Outdated Show resolved Hide resolved
bulletin_board/server/README.md Outdated Show resolved Hide resolved
bulletin_board/server/README.md Outdated Show resolved Hide resolved
bulletin_board/server/README.md Outdated Show resolved Hide resolved
bulletin_board/server/README.md Outdated Show resolved Hide resolved
bulletin_board/server/README.md Outdated Show resolved Hide resolved
bulletin_board/server/README.md Outdated Show resolved Hide resolved
ports:
- "8000:8000"
depends_on:
- redis
Copy link
Member

Choose a reason for hiding this comment

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

As this is for production, shouldn't we add restart: unless-stopped for all the services?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, although I imagine a real production docker-compose pulling a image directly from dockerhub instead of building it here. Let's say that this also work for development

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, production configurations can be very personal. I imagine most people would use a Nginx service integrated here instead of exposing the 8000 port form rails.

bulletin_board/server/README.md Outdated Show resolved Hide resolved
@andreslucena
Copy link
Member

Also, when the notion.so manuals are migrated to docs/ and https://docs.decidim.org/bulletin-board/, then we should link at the beginning and the ending of this guide to have a full detailed explanation, but I really like having the TLDR in the README 👍🏽 👍🏽

microstudi and others added 7 commits November 25, 2021 15:44
Co-authored-by: Andrés Pereira de Lucena <[email protected]>
Co-authored-by: Andrés Pereira de Lucena <[email protected]>
Co-authored-by: Andrés Pereira de Lucena <[email protected]>
Co-authored-by: Andrés Pereira de Lucena <[email protected]>
Co-authored-by: Andrés Pereira de Lucena <[email protected]>
Co-authored-by: Andrés Pereira de Lucena <[email protected]>
Co-authored-by: Andrés Pereira de Lucena <[email protected]>
@microstudi
Copy link
Contributor Author

Also, when the notion.so manuals are migrated to docs/ and https://docs.decidim.org/bulletin-board/, then we should link at the beginning and the ending of this guide to have a full detailed explanation, but I really like having the TLDR in the README 👍🏽 👍🏽

This are really good news!

Copy link
Contributor Author

@microstudi microstudi 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 the fixes @andreslucena, I let you a couple of comments.

ports:
- "8000:8000"
depends_on:
- redis
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, production configurations can be very personal. I imagine most people would use a Nginx service integrated here instead of exposing the 8000 port form rails.

@ahukkanen
Copy link
Contributor

@microstudi Going through these open issues at bulletin board. Are you planning to continue working on this or should we close it?

We would need to rebase/merge with develop and address the possible feedback. I haven't tried out the docker compose setup yet because I am unsure if you will continue working on this.

@microstudi
Copy link
Contributor Author

We should close this one in favor of #309

@andreslucena
Copy link
Member

Closing this one as we're moving forward with #309

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants