Skip to content
This repository has been archived by the owner on Dec 7, 2023. It is now read-only.

add .editorconfig file #106

Merged
merged 3 commits into from
Dec 18, 2019
Merged

Conversation

gggeek
Copy link
Contributor

@gggeek gggeek commented Dec 16, 2019

Description

Add an editorconfig file

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I updated the documentation (see here)
  • I agree that this code is used in AdminLTEBundle and will be published under the MIT license

Copy link
Owner

@kevinpapst kevinpapst left a comment

Choose a reason for hiding this comment

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

While I do agree with most of the settings, I I don't feel completely comfortable with the last two ones.
The trim trailing whitespace can introduce weird problems at least when editing the documentation .md files.

Probably move the last two rules under a *.php block?!

[*.php]
insert_final_newline = true
trim_trailing_whitespace = true

@gggeek
Copy link
Contributor Author

gggeek commented Dec 16, 2019

OK.
Are you a fan of 'rebase your PRs so that they only show 1 commit', or is it ok to have many commits in your git logs?

@gggeek
Copy link
Contributor Author

gggeek commented Dec 16, 2019

I moved the more 'aggressive' trimming and newline settings in a dedicated block, but made it apply to most file types besides php.
In my own experience, I never had troubles with that, as long as there is a minimum of discipline applied throughout the codebase (arguably, one could create even a php file where trailing whitespace is semantically important, but I am sure such practice is universally considered bad)

@kevinpapst
Copy link
Owner

OK.
Are you a fan of 'rebase your PRs so that they only show 1 commit', or is it ok to have many commits in your git logs?

I don't care, I use squash merging anyway ;-)

Regarding the rules: I want to avoid that someone pushes a PR that suddenly changes whitespaces ... you are right that it shouldn't make a difference. But this repo was forked and some of the code is pretty old and wasn't touched by myself.

For PHP files I agree that it is safe, as automatic rules via phpcs are already in place.
Conclusion 😁 please use [*.php] only for now.

@gggeek
Copy link
Contributor Author

gggeek commented Dec 18, 2019

updated

@kevinpapst kevinpapst merged commit 36def0e into kevinpapst:master Dec 18, 2019
@kevinpapst
Copy link
Owner

Thank you @gggeek 👍

kevinpapst added a commit that referenced this pull request Jan 7, 2020
* added .editorconfig file (#106)

* Updating to Bootstrap 4 and AdminLTE3. Mostly Complete, Some Customization missing .

* Fix test

* fix auth screens

* removed incompatible settings

* added upgrading docs

* badge color name change for menu items

* rebuild assets with some new styles

* many improvements for admin lte v3

* improve sidebar for one tab

* form types migrated to bootstrap 4

* fix tests

Co-authored-by: Gaetano Giunta <[email protected]>
Co-authored-by: Kevin Papst <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants