Skip to content

Conversation

@faysou
Copy link
Collaborator

@faysou faysou commented Oct 10, 2025

Pull Request

NautilusTrader prioritizes correctness and reliability, please follow existing patterns for validation and testing.

  • I have reviewed the CONTRIBUTING.md and followed the established practices

Summary

It's better to set env variables using the make syntax so it works on windows as well.

Related Issues/PRs

Type of change

  • Bug fix (non-breaking)
  • New feature (non-breaking)
  • Breaking change (impacts existing behavior)
  • Documentation update
  • Maintenance / chore

Breaking change details (if applicable)

Documentation

  • Documentation changes follow the style guide (docs/developer_guide/docs.md)

Release notes

  • I added a concise entry to RELEASES.md that follows the existing conventions (when applicable)

Testing

Ensure new or changed logic is covered by tests.

  • Affected code paths are already covered by the test suite
  • I added/updated tests to cover new or changed logic

Copy link
Member

@cjdsellers cjdsellers left a comment

Choose a reason for hiding this comment

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

Thanks @faysou, good refinements.

I'll fix those two small things on develop branch.

$Q uv sync --active --all-groups --all-extras --verbose

.PHONY: install-debug
install: BUILD_MODE=debug
Copy link
Member

Choose a reason for hiding this comment

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

This needs to be install-debug

.PHONY: build-wheel
build-wheel: BUILD_MODE=release
build-wheel: #-- Build wheel distribution in release mode
BUILD_MODE=release uv build --wheel
Copy link
Member

Choose a reason for hiding this comment

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

This could be removed now as well

@cjdsellers cjdsellers merged commit 2624408 into develop Oct 10, 2025
17 checks passed
@cjdsellers cjdsellers deleted the makefile branch October 10, 2025 20:42
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.

3 participants