dagster-io / dagster-cloud-action

Apache License 2.0
13 stars 20 forks source link

add conventional style guardrails #203

Open dbrtly opened 3 weeks ago

cmpadden commented 1 week ago

Thanks, @dbrtly .

@alangenfeld, was hoping to get your opinion on this? My only concern is that it's possible the editor config might conflict with tools like Ruff, what do you think?

alangenfeld commented 1 week ago

I haven't spent much time on this stuff - I don't love adding another place where these values are configured without some way to enforce them staying in sync.

@smackesey you have thoughts here?

dbrtly commented 1 week ago

Like Black, automatically detect the appropriate line ending. https://docs.astral.sh/ruff/configuration/#__codelineno-0-64 line-ending = "auto"

https://docs.astral.sh/ruff/configuration/

I agree that in principal if ruff can configure all these it would be better to use Ruff with the default line ending configuration. Prompted me to learn more about ruff. What about the non-py-ecosystem files though? I would not expect them to be in scope for ruff.

IMHE line ending weirdness is a such a pain (time and frustration) that I pre-emptively add the gitattributes to avoid ever going through it again. Git detects phantom changes that cannot be discarded easily.

For example astral has added these files here https://github.com/astral-sh/ruff/tree/main and https://github.com/astral-sh/uv/tree/main

Even though line endings are configurable in rust: https://rust-lang.github.io/rustfmt/https://rust-lang.github.io/rustfmt/?version=v1.6.0&search=#newline_style?version=v1.6.0&search=#newline_stylehttps://rust-lang.github.io/rustfmt/?version=v1.6.0&search=#newline_style

Interested in your perspective.

Thanks, Daniel


From: Alex Langenfeld @.> Sent: Friday, November 1, 2024 1:50:26 AM To: dagster-io/dagster-cloud-action @.> Cc: Daniel Bartley @.>; Mention @.> Subject: Re: [dagster-io/dagster-cloud-action] add conventional style guardrails (PR #203)

I haven't spent much time on this stuff - I don't love adding another place where these values are configured without some way to enforce them staying in sync.

@smackeseyhttps://github.com/smackesey you have thoughts here?

— Reply to this email directly, view it on GitHubhttps://github.com/dagster-io/dagster-cloud-action/pull/203#issuecomment-2450080509, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ADLXPTOF6IKS24TGG5CFDY3Z6I7TFAVCNFSM6AAAAABQG6M7PSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINJQGA4DANJQHE. You are receiving this because you were mentioned.Message ID: @.***>