Repository navigation
Conversation
The two attribute writers were the last tag gaps in the driver: workflow_name= had no @PARAM and workflow_id= had no @return. Neither could be fixed in place, because YARD shares one docstring between the reader and the writer of an attr_accessor, so a @PARAM added there is reported as an unknown parameter name on the reader. Split the accessor into an attr_reader and an attr_writer per attribute so each direction carries its own tags, and note on both writers that set_workflow_vars is the intended way to switch workflows since it also resets the memoized client and output parameters. Runtime behaviour is unchanged: attr_reader plus attr_writer defines the same two methods attr_accessor did. Signed-off-by: Tim Smith <tim@mondoo.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
yard statshas reported 100.00% documented for a while, but object coverageonly asks whether an object has a docstring -- it says nothing about whether
the tags inside are complete. Auditing the registry directly for missing
@param/@returntags turned up two gaps, both on the generated attributewriters:
Why the accessor is split
Neither gap could be closed in place. YARD shares a single docstring between the
reader and the writer of an
attr_accessor, so:@param valueadded to that shared docstring lands on the reader too,which takes no arguments -- YARD then emits
@param tag has unknown parameter name: value(verified: adding it produced12 such warnings), and
@!attributedirective form is used, YARD replaces the writer'sdocstring with an auto-generated
@param valueand the@returnis lost --which is exactly the asymmetry the audit found between
workflow_name=andworkflow_id=.So this PR declares an
attr_readerand anattr_writerper attribute insteadof one
attr_accessor, which lets each direction carry its own tags. Thisdefines exactly the same two methods
attr_accessordid; there is no runtimechange. A short comment records why the pair is written out, so it does not get
"tidied" back into an accessor and silently reopen the gap.
While there, both writers now note that {#set_workflow_vars} is the intended way
to switch workflows, since it also clears the memoized client and the memoized
output parameters. Assigning the attribute on its own leaves both pointing at the
previous workflow -- that is a real footgun the existing docs on
set_workflow_varsdescribe but the writers themselves did not.What this PR does not do
.yardopts. Both already exist andboth still work:
rake -Tlistsrake docandrake doc_coverage, and bothstill report
100.00% documentedwithAttributes: 2 (0 undocumented).Verification
No
@param tag has unknown parameter namewarnings remain:Those two are pre-existing and unrelated to Ruby docs:
LICENSE.txtis listed asan extra file in
.yardoptsand the Apache boilerplate's[yyyy]and[name of copyright owner]placeholders get parsed as markdown links. Left alonehere.
Docs coverage unchanged:
Tests, same count before and after:
Lint clean:
Worth noting:
mainis green under Cookstyle 9.0.0 again. TheLayout/ExtraSpacingoffense on the unusedwebmockline is gone, because #44removed the
webmockdependency entirely.Merge order
No conflicts expected. #44 and #45 have both landed, and this branch is cut from
the current
main. #46 (ci:) touches workflow files rather thanlib/, so thetwo are independent. #7 is an outside contribution that is already unmergeable
against
mainand will need rebasing regardless of this PR; it does not touchthese attribute declarations.
Unrelated, for a separate PR
This repo's licence file is
LICENSE.txt, while 18 of the 21 sibling repos useLICENSE. As a result the README's[LICENSE](LICENSE)link (README.md:257)404s. Deliberately not touched here -- flagging it so it can be handled on its
own.