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

chore(housekeeping): Normalize how config files filter out unwanted lines #2619

Conversation

polarathene
Copy link
Member

@polarathene polarathene commented Jun 5, 2022

Description

We have used various ways to filter out unwanted lines (empty, white-space only, comments) from configs when reading them to iterate through. In some cases, we had not filtered for all kinds of unwanted lines, or did not filter at all (even though we probably should have).

This PR has normalized that approach to call a helper method instead (added in #2617), with a common approach to call the filter method applied:

  • The changes are mostly converting config file paths into local vars to reference instead. These are staged out into commits for easy review if needed (no commit messages).
  • Then normalizes the config file pre-processing step to use the recently added method _get_valid_lines_from_file() (renamed in this PR from earlier _filter_to_valid_lines() due to feedback in #2617 ).
  • helpers/relay.sh had a loop that only seems to strip out the unwanted lines for the target config. This doesn't seem necessary and could probably just be a cp instead? I collapsed that loop to call the filtering method and just write the output of that instead.
  • helpers/utils.sh:_is_comment() has been removed, as only relay.sh was using it and it no longer seems to serve any purpose.

Relevant history

Looks like initial filtering to avoid issues was introduced via this PR in May 2020.

Type of change

  • 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

@polarathene polarathene added area/scripts kind/improvement Improve an existing feature, configuration file or the documentation pr/waiting for other PR to get merged This PR is waiting for another / other PR(s) to get merged labels Jun 5, 2022
@polarathene polarathene added this to the v11.1.0 milestone Jun 5, 2022
@polarathene polarathene self-assigned this Jun 5, 2022
@polarathene polarathene force-pushed the chore/housekeeping-parsing-functions branch from 07582bb to 3b66022 Compare June 5, 2022 23:33
@polarathene polarathene removed the pr/waiting for other PR to get merged This PR is waiting for another / other PR(s) to get merged label Jun 5, 2022
@polarathene polarathene marked this pull request as ready for review June 5, 2022 23:39
@polarathene polarathene requested review from georglauterbach and casperklein and removed request for georglauterbach June 5, 2022 23:39
@polarathene

This comment was marked as resolved.

@polarathene polarathene merged commit 54904aa into docker-mailserver:master Jun 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/scripts kind/improvement Improve an existing feature, configuration file or the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants