Skip to content

Updated ast_engine README.md#124

Merged
sdrichards-bc merged 2 commits into
mainfrom
add/ast-engine-readme
Jul 10, 2026
Merged

Updated ast_engine README.md#124
sdrichards-bc merged 2 commits into
mainfrom
add/ast-engine-readme

Conversation

@sdrichards-bc

Copy link
Copy Markdown
Collaborator

The README includes supporting guidance for configuration, developer setup, logging, error handling, test execution, Git branch workflow, and general development standards for the AST Engine package. Please add your documentation to the relevant section as development continues.

@MoezLabiadh MoezLabiadh left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good to me! The README is really useful. I just added a couple of comments on the File adapter (kml adapter no longer exists) and running tests (markers...)

Comment thread ast_engine/README.md Outdated
Comment thread ast_engine/README.md

This package uses `pytest`.

Run all tests:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think it is worth mentioning that tests carries the "unit" marker and convention is "uv run pytest -m unit" to run all "unit" tests. In the future Integration tests can carry a different marker and stay out of the default "unit" fast test runs.

Comment thread ast_engine/README.md Outdated
```text
ast_engine/tests/
├── unit/
├── integration/

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Also worth mentioning that integration tests are live-database checks and will need their own marker to seperate from unit tests. Today these are done with the smoke scripts under "scripts/" (currently only scripts/oracle_smoke.py, which runs the real Oracle adapter against BCGW) - credentials come from the BCGW_USER / BCGW_PASSWORD / BCGW_HOST environment variables, or a prompt, never hardcoded.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Added some additional notes about additional markers as dev progresses. Please have a look and adjust as needed once merged.

@jordanjfoy

Copy link
Copy Markdown
Collaborator

Awesome Job Steve, this looks really good and is really useful! Thanks!

@wburt

wburt commented Jul 6, 2026

Copy link
Copy Markdown
Member

Looks great. This triggers some changes to how we do things. Also... is uv pip install -e required for local dev use or do our tests etc work without that?

Comment thread ast_engine/README.md
```text
.venv/
```

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is uv pip install -e required for local dev use or do our tests etc work without that?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Not needed as uv sync projects are installed as editable packages, so changes to the project are available immediately when ran locally.

Comment thread ast_engine/README.md
Open a pull request into `main`.

### Pull Request Checklist

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is good. Creating a ticket to include this in a pull request form.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Awesome.

@sdrichards-bc
sdrichards-bc merged commit e655578 into main Jul 10, 2026
2 checks passed
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