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

ci(fix): Normalize for .gitattributes + improve eclint coverage #3566

Merged

Conversation

polarathene
Copy link
Member

@polarathene polarathene commented Oct 3, 2023

Description

This is a follow-up PR to #3527

Postgrey had an init script contributed in Feb 2017 as part of the feature implementation.

Additional context between the two files that have meaningful changes .ecrc.json and .editorconfig can be found in the commit messages of their individual commits.

For reference:

  • Default eclint configuration is here.
  • Default eclint excludes are listed here.
  • test/test-files is ignored, but it could be for the specific file affected if preferred. The failure is due to this upstream eclint bug. I've pinpointed the cause and deferred the fix to maintainers there.

Fixes #3565

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change that does improve existing functionality)

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • New and existing unit tests pass locally with my changes

`postfix-accounts.cf` in particular had a mix of `CRLF` and `LF`.
Better align with the `.editorconfig` files rules (_which `eclint` reads_).

Most of this is the upstream defaults and can be dropped.
- `SpaceAftertabs` (_possibly invalid name?_) was set to `true` but it doesn't appear any files covered rely on this. No context in `git blame` for it's addition, dropping.
- `Exclude` has a long history of pattern additions with minimal context. This is updated to be more specific now.
- `IgnoreDefaults` has been left for visibility. It could technically be set to `true`, but the SVG files should be ignored then.

`test/test-files/` is only ignored due to an upstream bug with `eclint` incorrectly detecting a file with the wrong encoding and triggering a failure as it's unsupported by `eclint`. Can be removed once a new release resolves that.

`test/bats/` and it's related submodules `bats-assert` / `bats-support` need to be ignored. In addition to not being relevant to the project, they supply their own `.editorconfig` with rules that seem to fail such as `max_line_length`.

`.git/` also isn't ignored by default (_it apparently was in an earlier version of `eclint` according to a bug report_).
We can bring the `indent_size = 2` rule into all files default. It helps establish a bit of consistency in the project files.

`.gitmodules` could possibly be ignored instead, it's also using tabs like `Makefile` so I've just configured that explicitly here.

The git submodules section doesn't seem relevant? Those changes shouldn't be committed, and we're not likely to modify those intentionally. The project supplies it's own `.editorconfig` file as root which would ignore this section, thus it's not really serving a purpose?
Most changes are for the `indent_size = 2` rule now having expanded relevance.

1 file needed a blank line `dovecot-master-accounts.md`, another had trailing white-space `mail_with_ldap.bats`.
Just white-space changes with indentation to be multiples of 2, and replace any tabbed indentation with spaces.
This has not been used since the switch to `supervisord`.
@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2023

Documentation preview for this PR is ready! 🎉

Built with commit: 84acba2

@georglauterbach georglauterbach merged commit aae42fa into docker-mailserver:master Oct 4, 2023
11 checks passed
@rasa
Copy link

rasa commented Oct 4, 2023

If my PR is accepted, editorconfig-checker/editorconfig-checker#252 will be fixed soon. @polarathene Thank you for the detailed error report and your patience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug report: CI failure for contributors.yml (_due to files with CRLF conflicting with .gitattributes_)
3 participants