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

Update build instructions following move to Poetry #410

Conversation

FroggyFlox
Copy link
Member

Fixes #405
@phillxnet, ready for review.

This pull request's proposal

Update our contribution instructions following our move to Poetry (04a7d21).
This PR also includes minor typos and style adjustments (a069da9) in an attempt at making the text more fluent and thus hopefully a bit easier to follow. I hope it actually achieves that.

@phillxnet, I've tried to separate the changes into two commits to ease review; commit 04a7d21 should include all changes related to the move to Poetry.

Checklist

  • With the proposed changes no Sphinx errors or warnings are generated.
  • I have added my name to the AUTHORS file, if required (descending alphabetical order).

Our current docs still describe our now-deprecated build method
based on buildout. This commit includes a series of changes to
describe our new build instructions based on Poetry:
  - modify missing build dependencies
  - use our new build.sh script
  - update new required source tree root (/opt/rockstor)
Harmonize use of GitHub vs Github.
Emphasize some important notes by moving them to note blocks.
Highlight paths by using `:code:` annotation.
Minor typos or style changes.
Copy link
Member

@phillxnet phillxnet left a comment

Choose a reason for hiding this comment

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

@FroggyFlox this is all a most welcome update. Thanks. And as always much appreciated.

I've popped in a few suggestions that I think would be nice-to-haves, see what you think.

Either way I think this is ready for PRODUCTION as the sooner we get this updated the better. I'll await your decisions on the suggestions and then I can publish there-after if you don't beat me to it.

contribute/contribute.rst Outdated Show resolved Hide resolved
contribute/contribute.rst Show resolved Hide resolved
contribute/contribute.rst Show resolved Hide resolved
Comment on lines +428 to +434
.. code-block:: text

buildvm:~ # cd /opt/rockstor
buildvm:~ # export DJANGO_SETTINGS_MODULE=settings
buildvm:~ # poetry run debug-mode ON

Everything that your code or any Rockstor service logs goes into the following files:
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for this. I'm surprised we didn't already have this!

Copy link
Member Author

Choose a reason for hiding this comment

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

Our buildout method actually automatically set DEBUG to True in settings.py. I forgot exactly how, but I remember seeing it. What do you think of adding an optional flag to build.sh to do that (leaving defaults as is, of course)?

Copy link
Member

Choose a reason for hiding this comment

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

What do you think of adding an optional flag to build.sh to do that (leaving defaults as is, of course)?

That sound like the way to go, given we use the same script when building the package. Plus it will 'surface' this debug element for those that read no further than the build.sh itself. Which brings to mind the idea that we include a referenced to our developer docs within build.sh itself. So folks can easily find this doc section with only a casual look at build.sh.

Copy link
Member Author

Choose a reason for hiding this comment

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

I do like the idea of adding a link to the docs in build.sh itself!

contribute/contribute.rst Outdated Show resolved Hide resolved
contribute/contribute.rst Outdated Show resolved Hide resolved
FroggyFlox and others added 3 commits January 30, 2023 09:12
Co-authored-by: Philip Guyton <[email protected]>
Specify that Rockstor's jslibs are unzipped into the final directory by the build script.

Co-authored-by: Philip Guyton <[email protected]>
Correct a remaining instance of the old `/opt/rockstor-dev` path to `/opt/rockstor`.

Co-authored-by: Philip Guyton <[email protected]>
@FroggyFlox
Copy link
Member Author

Thanks @phillxnet for your thorough review; I included all your suggestions.

@FroggyFlox FroggyFlox requested a review from phillxnet January 30, 2023 14:20
@phillxnet phillxnet merged commit dc75ed6 into rockstor:master Jan 30, 2023
@phillxnet
Copy link
Member

PR product PRODUCTION published.

@FroggyFlox FroggyFlox deleted the 405_Update_build_instructions_for_poetry_move branch January 30, 2023 15:10
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.

Update build instructions to reflect move to Poetry
2 participants