(CAT-1646) - Remove section if it has empty line but does not have any settings #532
Conversation
Ramesh7
changed the title
Ramesh7
force-pushed
the
CAT-1646-remove-section-when-its-empty
branch
3 times, most recently
from
42cfdb1
to
ac3f688
Compare
Ramesh7
force-pushed
the
CAT-1646-remove-section-when-its-empty
branch
from
ac3f688
to
4ebe030
Compare
Ramesh7
requested review from
bastelfreak,
ekohl,
smortex and
a team
as code owners
Agreed with you @bastelfreak, BTW it will just remove the empty section which has only new line but it won't impact if a section just has comment. So I feel it should be good to go. |
|
This is a breaking change and it needs to be an optional behavior. For example, this breaks splunk indexer configuration, which configures replication port with no other settings: I would have to use file_line now |
|
When I read #524 (which this PR is supposed to address and should have been closed?), I understand that the behavior was different when removing the last setting of a section depending on the presence or absence of blank lines in that section, and I guess the behavior is still different if the section has comments. Inconsistency is indeed not great. AFAIAC, a I believe we should revert this and release 6.1.2 to remove this breaking change. Then if we want to re-introduce this, a new major version will be required, and I would vote for changing the @Ramesh7, @vchepkov, @bastelfreak what do you think? |
|
I agree |
PR #532 introduced 4ebe030 breaks backwards compatibility and is part of a patch release, as stated here: #532 (comment) This reverts commit 4ebe030. This will allow us to do a new patch release that fix the unexpected backwards incompatible change, and leave the room to implement this as part of a next major version of the module.
PR #532 introduced 4ebe030 which breaks backwards compatibility and is part of a patch release, as reported here: #532 (comment) This reverts commit 4ebe030. This will allow us to do a new patch release that fix the unexpected backwards incompatible change, and leave the room to implement this as part of a next major version of the module.
When the last setting of a section was removed, the whole section was removed unless it contained white space of comments. In #532, this was changed to also remove sections that only contained space (blank lines), but it caused regressions and was reverted in #535. For consistency, we completely suppress the auto-removal of "empty" sections: removing the last setting of a section will not remove this section anymore, just like what happens for sections with only blank lines and comments.
When the last setting of a section was removed, the whole section was removed unless it contained white space of comments. In #532, this was changed to also remove sections that only contained space (blank lines), but it caused regressions and was reverted in #535. For consistency, we completely suppress the auto-removal of "empty" sections: removing the last setting of a section will not remove this section anymore, just like what happens for sections with only blank lines and comments.
When the last setting of a section was removed, the whole section was removed unless it contained white space of comments. In #532, this was changed to also remove sections that only contained space (blank lines), but it caused regressions and was reverted in #535. For consistency, we completely suppress the auto-removal of "empty" sections: removing the last setting of a section will not remove this section anymore, just like what happens for sections with only blank lines and comments.
When the last setting of a section was removed, the whole section was removed unless it contained white space of comments. In #532, this was changed to also remove sections that only contained space (blank lines), but it caused regressions and was reverted in #535. For consistency, we completely suppress the auto-removal of "empty" sections: removing the last setting of a section will not remove this section anymore, just like what happens for sections with only blank lines and comments.
When the last setting of a section was removed, the whole section was removed unless it contained white space of comments. In #532, this was changed to also remove sections that only contained space (blank lines), but it caused regressions and was reverted in #535. For consistency, we completely suppress the auto-removal of "empty" sections: removing the last setting of a section will not remove this section anymore, just like what happens for sections with only blank lines and comments.
|
I opened three PR that are ready for testing (each PR include the previous ones):
The last 2 are draft as they should not be merged before a new release including the first PR is done. However, they are ready for review. Thanks! |
Summary
Current implementation keeping section as it is if just contain empty line as described here, this change will delete the section if just contain empty line.
Related Issues (if any)
#524
Checklist
puppet apply)