some documentation clarifications, dev improvements

mathematicalmichael commented 1 year ago

looks like I need node to run pytest? I really don't like having node on my system, is it really required?

If so, I'm going to go grab a node/npm container from another one of my projects and bring it into here as a "fake binary" (shell script in PATH that runs docker command).

something like this:

FROM node:18-alpine                                                                            

RUN apk --no-cache add \
    shadow \                                                                   
    g++ \
    gcc \                                                                                         
    musl-dev \                                                                                    
    autoconf \                                                                                    
    automake \                                                                                    
    make \                                                                                        
    libtool \                                                                                     
    nasm \                                                                                        
    tiff \                                                                                        
    jpeg \                                                                                        
    zlib \                                                                                        
    zlib-dev \                                                                                    
    file \                                                                                        
    pkgconf \                                                                                     
    python3 \
    python3-dev \
    && yarn install

#!/bin/sh
IMAGE_NAME=node
COMMAND=node
docker run -ti --rm -v $(pwd):/temp -w /temp $IMAGE_NAME $COMMAND "$@"

mathematicalmichael commented 1 year ago

well this is annoying... I'm editing in vim, tried adding a newline, still getting failures in CI but on my machine the pre-commit worked fine..

[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
[INFO] Installing environment for https://github.com/pre-commit/pre-commit-hooks.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
[INFO] Installing environment for https://github.com/humitos/mirrors-autoflake.git.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
[INFO] Installing environment for https://github.com/akaihola/darker.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
update-projen-managed-files..........................(no files to check)Skipped
Check for merge conflicts............................(no files to check)Skipped
Trim Trailing Whitespace.............................(no files to check)Skipped
Fix End of Files.....................................(no files to check)Skipped
Check Toml...........................................(no files to check)Skipped
Check Yaml...........................................(no files to check)Skipped
Check for broken symlinks............................(no files to check)Skipped
Check for added large files..........................(no files to check)Skipped
Fix requirements.txt.................................(no files to check)Skipped
Forbid new submodules................................(no files to check)Skipped
Don't commit to branch...................................................Passed
Detect Private Key...................................(no files to check)Skipped
rst ``code`` is two backticks........................(no files to check)Skipped
rst directives end with two colons...................(no files to check)Skipped
rst ``inline code`` next to normal text..............(no files to check)Skipped
autoflake............................................(no files to check)Skipped
darker...............................................(no files to check)Skipped

mathematicalmichael commented 1 year ago

I'm guessing there's a reason the check in question is being skipped for me but not in CI.

oh... looks like it may be related to missing node... why do I need node to run python-related checks =/

mathematicalmichael commented 1 year ago

alright I installed node so I can run the checks, but it seems setup.cfg is "managed by projen" and so I can't be editing this file. but ... I don't see where some of the contents of setup.cfg are actually defined...

I'm just trying to change the URL here...

$ rg rootski
Justfile
27:    # # install git lfs for downloading rootski CSVs and other large files in the repo

awscdk-minecraft/setup.cfg
18:    Documentation = https://docs.rootski.io

so this seems to be a "default" inherited by the use of phito-projen

mathematicalmichael commented 1 year ago

yup. indeed. @phitoduck it seems like this line doesn't take documentation URL as an argument, so all projects will have the same "rootski.io" url in setup.cfg.

https://github.com/phitoduck/phito-projen/blob/ec59a20b00190d70cccca2b4bb5ca7ac4a451ec5/src/phito_projen/components/setup_cfg/setup_cfg.py#L38

mathematicalmichael commented 1 year ago

fyi conda install -c conda-forge just seems to not include just in my PATH... trying to figure out a non-brew installation.

this seems to work:

curl --proto '=https' --tlsv1.2 -sSf https://just.systems/install.sh | bash -s -- --to <DEST IN YOUR PATH>

for me DEST is ~/bin, which sits first in my PATH.

mathematicalmichael commented 1 year ago

alright I give up dockerizing this...

got so close, but the fact i'm running macOS means the path that's being requested for @jsii is mac-specific but my image is linux, so it couldn't find the right directory. If I ran everything in a container this would be okay, so I just added clarification to the README that node is required and how one can go about installing it on their machines..

brew for linux I suppose is an option.

mathematicalmichael commented 1 year ago

also, the justfile fails to run just install if you don't have code installed (which I do, but it seems to not be in my path).

putting in a quick fix for it by adding || exit 0

PR ready.

phitoduck commented 1 year ago

Wow... this is the tale of an epic struggle. Dang, I'm sorry all these things conflicted. node get's used in 3 places in this project:

The TypeScript/React app
AWS CDK (the Python AWS CDK library is really a set of bindings over the TypeScript CDK engine)
projen (same as this ^^^)

I'm running projen as a pre-commit hook. You probably figured that out already. For the sake of reducing the learning curve of the project, I'll take out projen.

We're getting a lot of value from the Justfile currently, but maybe I should find a way to remove that, too, in favor of make or something else.

phitoduck commented 1 year ago

K,

removed projen from the project
fixed linting issues (since it was previously not running due to a bug)
fixed an additional bug that caused linting to always fail

mlops-club / awscdk-minecraft

some documentation clarifications, dev improvements #5