Skip to content

add formatting#132

Merged
amirbilu merged 4 commits intocodota:masterfrom
aarondill:add-formatting
Nov 6, 2023
Merged

add formatting#132
amirbilu merged 4 commits intocodota:masterfrom
aarondill:add-formatting

Conversation

@aarondill
Copy link
Copy Markdown
Contributor

chore: add formatting workflow and stylua.toml

These should ensure consitent formatting in the future.
Feel free to modify the stylua.toml to your liking. I tried to keep it consistent with our current style (especially the tab-based indention)

These should ensure consitent formatting in the future.

Feel free to modify the stylua.toml to your liking. I tried to keep it consistent with our current style (especially the tab-based indention)
@aarondill
Copy link
Copy Markdown
Contributor Author

This is an implementation of this: #131 (comment)

Copy link
Copy Markdown
Contributor

@amirbilu amirbilu left a comment

Choose a reason for hiding this comment

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

This looks good. Do we really need both format and push and check on PR? Usually you pick one of them. No?

@aarondill
Copy link
Copy Markdown
Contributor Author

My thought process was that having a check on PRs encourages (but doesn't require?) contributors to run a formatter on their changes code to get the green check, thusly reducing diff sizes.

If you don't like this idea, I can remove the PR check and leave only the format-on-push.

@amirbilu
Copy link
Copy Markdown
Contributor

amirbilu commented Nov 6, 2023

With most Tabnine repos we just perform format check upon PR

@aarondill
Copy link
Copy Markdown
Contributor Author

With most Tabnine repos we just perform format check upon PR

So, to be clear, you don't want the auto-formatting at all? Just a check on new PRs? Wouldn't it slightly defeat the purpose of having a format workflow if it is still delegated to the contributor in all cases?

Additionally, wouldn't that have the potential to create further merge conflicts, since if a formatting mistake makes it to master (through a direct commit), it would then have to be fixed in every PR following, until one is merged?

@amirbilu
Copy link
Copy Markdown
Contributor

amirbilu commented Nov 6, 2023 via email

@amirbilu amirbilu merged commit 20c484f into codota:master Nov 6, 2023
@amirbilu
Copy link
Copy Markdown
Contributor

amirbilu commented Nov 6, 2023

Made this check mandatory

@aarondill aarondill deleted the add-formatting branch November 6, 2023 17:34
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