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

ref: improving development environment and instructions #5180

Merged
merged 4 commits into from
Dec 12, 2024

Conversation

mieslep
Copy link
Contributor

@mieslep mieslep commented Dec 10, 2024

This PR improves the ability for contributors to build and test changes, including better support for Windows users.

  • devcontainer more closely-aligned with CI/CD container definition
  • Details on how to contribute moved to DEVELOPMENT.md and expanded
  • Validated instructions on fresh WSL Ubuntu 22.04 installation
  • Minor adjustments to Makefile

@dosubot dosubot bot added the size:XL This PR changes 500-999 lines, ignoring generated files. label Dec 10, 2024
@mieslep mieslep changed the title improving development environment and instructions ref: improving development environment and instructions Dec 10, 2024
DEVELOPMENT.md Outdated

### Option 1 (Preferred): Use a Dev Container

Open this repostiory as a Dev Container per your IDEs instructions.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Open this repostiory as a Dev Container per your IDEs instructions.
Open this repository as a Dev Container per your IDEs instructions.

DEVELOPMENT.md Outdated
* **`make`**: The project uses `make` to coordidinate packaging.
* **`uv`**: This project uses `uv` (`>=0.4`), a Python package and project manager from Astral. Install instructions at https://docs.astral.sh/uv/getting-started/installation/.
* **`npm`**: The frontend files are built with Node.js (`v22.12 LTS`) and `npm` (`v10.9`). Install instrucations at https://nodejs.org/en/download/package-manager.
* Windows (WSL) users: ensure `npm` is installed within WSL environment; `which npm` should resolve to a Linux location, not a Windows location.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe there's an extra space here?

Suggested change
* Windows (WSL) users: ensure `npm` is installed within WSL environment; `which npm` should resolve to a Linux location, not a Windows location.
* Windows (WSL) users: ensure `npm` is installed within WSL environment; `which npm` should resolve to a Linux location, not a Windows location.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've switched this to a -, it is meant to be an indented sublist as it applies to npm but only Windows users.

Copy link
Contributor

@ogabrielluiz ogabrielluiz left a comment

Choose a reason for hiding this comment

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

Hey @mieslep

Incredible. This looks great!

Copy link
Contributor

Choose a reason for hiding this comment

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

One thing I would add to this is that we have debug configuration for VSCode and the developer can run it by pressing F5 in the editor or going to the debug tab.

@mieslep
Copy link
Contributor Author

mieslep commented Dec 12, 2024

@ogabrielluiz edits pushed!

@ogabrielluiz ogabrielluiz requested a review from Copilot December 12, 2024 11:39
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Dec 12, 2024
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 4 out of 8 changed files in this pull request and generated 1 suggestion.

Files not reviewed (4)
  • .devcontainer/Dockerfile: Language not supported
  • .devcontainer/demo/devcontainer.json: Language not supported
  • .devcontainer/devcontainer.json: Language not supported
  • Makefile: Language not supported

@ogabrielluiz ogabrielluiz merged commit d1821cb into langflow-ai:main Dec 12, 2024
7 checks passed
@mieslep mieslep deleted the phil/windows-dev branch December 12, 2024 12:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm This PR has been approved by a maintainer size:XL This PR changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants