Skip to content

Conversation

@anarchivist
Copy link
Member

also adds license file and updates link to B-Drive docs

@anarchivist anarchivist force-pushed the AP-496 branch 2 times, most recently from 8ffcba0 to dfd6302 Compare January 7, 2026 23:17
also adds license file and updates link to B-Drive docs
Copy link
Contributor

@steve-sullivan steve-sullivan left a comment

Choose a reason for hiding this comment

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

Looks good

Copy link
Member

@danschmidt5189 danschmidt5189 left a comment

Choose a reason for hiding this comment

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

Minor suggestion / rationale around declaring environment variables, but besides that looks good!

image: ${DOCKER_APP_IMAGE}
environment: !override
LDAP_HOST: ldap.fake.edu
LDAP_PASS: FAKEPW
Copy link
Member

Choose a reason for hiding this comment

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

It's not a hard requirement, but my suggestion is to standardize on the list / key=val syntax wherever possible because:

  1. The merge logic is effectively identical between the dict/list formats, so you don't lose anything.
  2. You gain the ability to copy/paste directly into Docker CLI commands.
  3. Because key=val forces YAML to parse everything as a string (which is what these always are), you can avoid some edge cases around, for example, a boolean true versus string "true".

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed in 407cd43

@anarchivist anarchivist merged commit f0d37fc into main Jan 8, 2026
5 checks passed
@anarchivist anarchivist deleted the AP-496 branch January 8, 2026 00:33
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.

4 participants