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

Add line splitting and other linting #1161

Merged
merged 1 commit into from
May 2, 2024
Merged

Add line splitting and other linting #1161

merged 1 commit into from
May 2, 2024

Conversation

b-chu
Copy link
Contributor

@b-chu b-chu commented May 1, 2024

Adds line splitting and trailing commas similar to mosaicml/composer#3078

@b-chu b-chu requested a review from a team as a code owner May 1, 2024 20:20
@b-chu b-chu requested review from mvpatel2000 and dakinggg May 1, 2024 20:20
Copy link
Collaborator

@mvpatel2000 mvpatel2000 left a comment

Choose a reason for hiding this comment

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

@b-chu should we raise min line length to be 100? it seems smaller than composer? or maybe im dumb.

I think looks good to me, would like daniel's pass too

llmfoundry/__init__.py Outdated Show resolved Hide resolved
@b-chu b-chu force-pushed the brian/lint branch 2 times, most recently from b1463a1 to f988c08 Compare May 2, 2024 15:41
@b-chu
Copy link
Contributor Author

b-chu commented May 2, 2024

Yes foundry is 80 but composer is 120. There are many other style inconsistencies between the two repos :) I'm going to say that's out of the scope of this PR and we should discuss how/if we want to reconcile.

@b-chu b-chu force-pushed the brian/lint branch 5 times, most recently from cc15f23 to 1d4b71c Compare May 2, 2024 19:46
@b-chu b-chu requested a review from mvpatel2000 May 2, 2024 19:48
@dakinggg dakinggg merged commit 10b7caf into main May 2, 2024
9 checks passed
@b-chu b-chu deleted the brian/lint branch May 2, 2024 20:25
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.

None yet

3 participants