Skip to content

Andrew/documentation strategy ldd#227

Merged
ayjayt merged 8 commits intomainfrom
andrew/documentation-strategy-ldd
Apr 16, 2025
Merged

Andrew/documentation strategy ldd#227
ayjayt merged 8 commits intomainfrom
andrew/documentation-strategy-ldd

Conversation

@ayjayt
Copy link
Copy Markdown
Collaborator

@ayjayt ayjayt commented Apr 15, 2025

Don't try to inject deps, just include better documentation.

@ayjayt ayjayt requested a review from emilykl April 15, 2025 19:54
Comment thread choreographer/browsers/_errors.py Outdated
"If you have already run the above command and are still "
"seeing this error, or the above command fails, consult the "
"Kaleido documentation for next steps: "
"https://path/to/docs/page/about/installing/dependencies\n"
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.

TODO: Update this link -- maybe create a INSTALLATION.md doc in the repo to link to for now?

Otherwise remove the link for now and can add it back later once there's something to link to.

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.

yeah that's right, dumb of me

Comment thread choreographer/browsers/chromium.py Outdated
return True

def _need_libs(self) -> bool: # noqa: C901 complexity
def _verify_libs(self) -> bool:
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.

needs docstring explaining what False/True return values mean for this function -- looks like False means "dependencies OK" and True means "dependencies missing OR ldd failed" ?

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.

yeah true

Copy link
Copy Markdown
Collaborator

@emilykl emilykl left a comment

Choose a reason for hiding this comment

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

🚀

@ayjayt ayjayt requested a review from emilykl April 16, 2025 21:16
@ayjayt ayjayt merged commit 6f94e1d into main Apr 16, 2025
4 checks passed
@ayjayt ayjayt deleted the andrew/documentation-strategy-ldd branch April 16, 2025 22:03
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.

2 participants