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

Guard coding style with autoformatter #125

Open
raboof opened this issue Jun 21, 2019 · 4 comments
Open

Guard coding style with autoformatter #125

raboof opened this issue Jun 21, 2019 · 4 comments

Comments

@raboof
Copy link
Owner

raboof commented Jun 21, 2019

The Notion coding style is described at http://notion.sourceforge.net/notionnotes/node7.html .

Unfortunately, it is not applied consistently, and a bit unorthodox.

It would be good to follow a consistent coding style, but I think it should not be manually enforced: we should just have an autoformatter.

This might be a good opportunity to also switch to a more conventional coding style.

I've had reasonably good experience with clang-format (with the default settings) in other projects, perhaps we should adopt that/those?

@wilhelmy
Copy link
Collaborator

wilhelmy commented Jul 1, 2019

The problem with adopting a new coding style (at least if we convert everything that already exists) is that we lose git blame-ability. So I'm in favour of using something more widely known for new files (and maybe eben on a function level), but not for a global autoformat run.

@raboof
Copy link
Owner Author

raboof commented Jul 2, 2019

we lose git blame-ability

I don't think that is a big problem: it may be a bit less convenient, but the GitHub 'Git Blame' functionality has the 'View blame prior to this change' buttons feature which makes this pretty easy to navigate. I suspect there's git clients who can do this as well, though I don't use those myself.

So I'm in favour of using something more widely known for new files (and maybe eben on a function level), but not for a global autoformat run

I think it is better to be consistent.

When we reformatted https://github.com/akka/akka the main nuisance was that it (as expected) created conflicts for in-flight PR's. The fact that 'git blame' became a bit harder hasn't been a problem.

@wilhelmy
Copy link
Collaborator

wilhelmy commented Jul 2, 2019

After 4.0, then, when we finished off all open PRs?

@raboof
Copy link
Owner Author

raboof commented Jul 2, 2019

After 4.0, then, when we finished off all open PRs?

Sure, that could be good timing! I don't like introducing dependencies between tasks, so:

  • I volunteer to fix any merge conflicts whenever this issue has been picked up and merged.
  • I think this issue can be, but doesn't have to be, picked up before cutting Notion 4.

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

No branches or pull requests

2 participants