Skip to content

Conversation

@haydenbaker
Copy link
Contributor

@haydenbaker haydenbaker commented Feb 24, 2025

Description of changes:

TODOs:

  • There are few TODOs after this PR is merged
    • We can add more linting rules as we see fit, I added a list near the TODO in the root pyproject.toml file
    • Update the README with a small guide on uv
    • Update the README with a section on the makefile

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@haydenbaker haydenbaker requested a review from a team as a code owner February 24, 2025 17:18
Comment on lines 6 to 8
authors = [
{ name = "Hayden Baker", email = "[email protected]" }
]
Copy link
Contributor

Choose a reason for hiding this comment

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

Bold, but not what we want here. iirc this is optional so best to remove it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Haha, yeah, this was a mistake. UV added this when initializing the projects.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we want to add any other fields from the previous toml files? e.g. license, keywords, classifiers, etc?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah let's add those, but as a separate PR so that this can get merged

@@ -1,34 +0,0 @@
name: Lint code
Copy link
Contributor

Choose a reason for hiding this comment

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

These have been removed, but it's not clear that they've been replaced. Have they?

Copy link
Contributor Author

@haydenbaker haydenbaker Feb 25, 2025

Choose a reason for hiding this comment

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

The main ci workflow does everything now, this includes linting/typechecking/tests/etc. This has the benefit of only requiring a single action, and only needing to set up the environment once. It runs fairly quickly (~3 minutes, most of which is spent initializing gradle and building java).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could separate out building codegen as a separate job within in the same action, but let's see if the action run time becomes an issue first.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not really concerned about runtime, i'm more concerned about separation of concerns. It's useful to see at a glance what's failing and why without having to dig into some logs. This isn't a blocker though - we can make that change later if we want

@haydenbaker haydenbaker force-pushed the haydenbaker/reorg-python branch from f45fdf6 to fc03d0a Compare February 25, 2025 00:31
@haydenbaker haydenbaker force-pushed the haydenbaker/reorg-structure branch from 71c6fde to cff5f59 Compare February 25, 2025 00:34
b"\x00\x00\x00\x00" # headers length
b"\x05\xc2\x48\xeb" # prelude crc
b"\x7D\x98\xc8\xff" # message crc
b"\x7d\x98\xc8\xff" # message crc
Copy link
Contributor

Choose a reason for hiding this comment

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

Dang, ruff is really this pedantic huh

@JordonPhillips JordonPhillips merged commit 1bbe2d4 into smithy-lang:haydenbaker/reorg-structure Feb 25, 2025
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