Reformatted standalone policy files with cfengine format#3675
Reformatted standalone policy files with cfengine format#3675olehermanse wants to merge 3 commits into
Conversation
|
@cf-bottom jenkins, please |
|
Alright, I triggered a build: Jenkins: https://ci.cfengine.com/job/fast-build-and-deploy-docs-master/63/ Documentation: http://buildcache.cfengine.com/packages/build-documentation-pr/jenkins-fast-build-and-deploy-docs-master-63/output/_site/ |
olehermanse
changed the title
olehermanse
requested review from
craigcomstock and
nickanderson
and removed request for
craigcomstock
|
@cf-bottom jenkins, please |
|
Alright, I triggered a build: Jenkins: https://ci.cfengine.com/job/fast-build-and-deploy-docs-master/64/ Documentation: http://buildcache.cfengine.com/packages/build-documentation-pr/jenkins-fast-build-and-deploy-docs-master-64/output/_site/ |
craigcomstock
left a comment
craigcomstock
left a comment
There was a problem hiding this comment.
I understand that comment placement is tricky so am approving this PR even though I found several places that seem to need more work to transition from old policy to formatted policy with comments.
Signed-off-by: Ole Herman Schumacher Elgesem <ole@northern.tech>
|
@cf-bottom jenkins, please |
|
Alright, I triggered a build: Jenkins: https://ci.cfengine.com/job/fast-build-and-deploy-docs-master/71/ Documentation: http://buildcache.cfengine.com/packages/build-documentation-pr/jenkins-fast-build-and-deploy-docs-master-71/output/_site/ |
olehermanse
force-pushed
the
master
branch
3 times, most recently
from
45bf592 to
26f7337
Compare
Signed-off-by: Ole Herman Schumacher Elgesem <ole@northern.tech>
Generally we should avoid commented out code; - It's not checked for syntax errors. - It will become outdated. - It is often not clear why it was there or why it was commented out. - It adds noise for the person reading the rest of the file. If you absolutely want to make a block of commented out code, you should keep these things in mind and probably: - Add comments to overcommunicate and make it very clear what it is and why it was commented out. - Add some tickets / TODOs for how we want to address it in the future. Signed-off-by: Ole Herman Schumacher Elgesem <ole@northern.tech>
nickanderson
left a comment
nickanderson
left a comment
There was a problem hiding this comment.
I don't really have a strong opinion about the line suffixed comments moving before/after. I think before makes a bit more sense, but after really isn't the end of the world.
| # The back reference in a path only applies to the last link | ||
| # of the pathname, so the (tmp) gets ignored | ||
| "/tmp/(cf3)_(.*)" edit_line => myedit("second $(match.2)"); | ||
| # but ... |
There was a problem hiding this comment.
Why did these comments bump out? Shouldn't #but ... and the others below be aligned with the comments before the promise since we are still inside the files promise block?
| replace_value => "/* $(match.1) */"; # backreference 0 | ||
| occurrences => "all"; # first, last all | ||
| replace_value => "/* $(match.1) */"; | ||
| # backreference 0 |
There was a problem hiding this comment.
Shouldn't these line suffix comments move to the line prior?
| create => "true", | ||
| edit_defaults => empty_file, # defined in stdlib | ||
| edit_line => create_solaris_admin_file; # defined in stdlib | ||
| # empty_file and create_solaris_admin_file are defined in stdlib: |
There was a problem hiding this comment.
I'm impressed that the format command constructed that from #defined in stdlib
4 participants
No description provided.