Comments (18)
I think this is a more broad issue than just in the SoftLayer::ImageTemplate class. Basically anywhere where the sl_attr method is used with the second parameter ala:
sl_attr :global_id, 'globalIdentifier'
Then in addition to rdoc'g the first parameter (e.g., "global_id" in the above) as an attribute, it will rdoc the second as well as an attribute.
from softlayer-ruby.
This kind of thing (from for example the SoftLayer::ImageTemplate class:
##
# :attr_reader:
# The notes, if any, that are attached to the template. Can be nil.
sl_attr :notes, "note"
causes rdoc to treat everything after "sl_attr" as an attribute. I think this will address this:
##
# :attr_reader: notes
# The notes, if any, that are attached to the template. Can be nil.
sl_attr :notes, "note"
But if it's somehow the intent that both "note" and "notes" are actually intended to be attributes then this is not the fix.
Can anyone comment on the intent here? I will submit a patch to fix up the rdoc defs here in the meantime, hoping that that's the way we want to go here...
from softlayer-ruby.
Right its only the first that should be the attribute. Ive been testing and we need to do this for all the objects for all sl_attr
and sl_dynamic_attr
entries.
from softlayer-ruby.
I think it should be this:
##
# :attr_reader: attr_label
sl_attr :attr_label, 'alt_attr_label'
##
# :method: dymanic_attr_label
# :call-seq:
# dynamic_attr_label(force_update=false)
#
# Description
sl_dynamic_attr :dynamic_attr_label do |param|
#body
end
[Edit] All sl_dynamic_attr
do seem to support the force_update
parameter, so these should be marked as :method:
and not :attr_reader:
and if it doesnt have a :call-seq:
section then it needs that as well.
from softlayer-ruby.
So on the sl_dynamic_attr
given that these are actually attributes, I guess it depends on taste how we want to really doc this, if we want to leave it as :attr_reader:
it doesnt seem to support :call-seq:
but we can indent a block above the description as follows:
##
# :attr_reader: dymanic_attr_label
# dynamic_attr_label(force_update=false)
#
# Description
sl_dynamic_attr :dynamic_attr_label do |param|
#body
end
The two space indent causes the method to appear in a "code" block as it does in normal markdown.
[Edit] Atlernative two:
##
# :attr_reader: dynamic_attr_label(force_update=false)
#
# Description
sl_dynamic_attr :dynamic_attr_label do |param|
#body
end
Would have to drop any :call-seq:
sections though.
from softlayer-ruby.
@ju2wheels - You wrote:
[...] we need to do this for all the objects for all sl_attr [...] entries.
For sure we should do this for any sl_attr that has the 2nd argument... Do you think we should specify the rdoc :attr_reader: modifier even for those that don't specify the 2nd argument, i.e., like this case?
##
# :attr_reader:
sl_attr :attr_label
from softlayer-ruby.
I dont see a need to unless theres a case where its showing up different in the doc like the two arg one.
from softlayer-ruby.
Also I notice that Datacenter.rb seems to have undocumented attributes. Is this on purpose or an oversight?
In Datacenter.rb:
sl_attr :name
sl_attr :long_name, "longName"
from softlayer-ruby.
Oversight, if its there it should be doc'ed as usable.
from softlayer-ruby.
@ju2wheels - I think I like the option where you specify the calling sequence directly on the :attr_reader: directive, what you called alternative 2, ala:
##
# :attr_reader: dynamic_attr_label(force_update=false)
#
# Description
sl_dynamic_attr :dynamic_attr_label do |param|
# body
end
That comes out looking like this which I prefer:
Would you be ok with going that way?
Does every sl_dynamic_attr defined attribute support the force_update=false argument? I think from looking at DynamicAttribute.rb that they do. I only see it documented in some of the dynamic attribute cases though...
from softlayer-ruby.
Yes all of them should support it, I tested it on a dynamic attribute that didnt have it doc'ed and it worked. Its hard to tell from the way the metaprogramming code was written but the getter it defines seems to consistently support that option.
Ill defer to Scott on the doc part for dynamic attrib, I could make the argument for either but given the implementation alt 2 makes the most sense.
from softlayer-ruby.
@SLsthompson - Could I trouble you to weigh in here on a couple of things about how we want to document the attributes created via sl_dynamic_attr?
-
How to rdoc sl_dynamic_attr as attributes? I guess these are unusual in that they are attributes but they take this force_update=true|false optional argument. @ju2wheels made some suggestions as to how to handle this above. I favor just rdoc'g them with the :attr_reader: modifier specifying the full method call ala:
## # :attr_reader: dynamic_attr_label(force_update=false) # # Description sl_dynamic_attr :dynamic_attr_label do |param| # body end
This would show the rather unusual "dynamic_attr_label(force_update=false)" in the rdoc attributes section when almost always Ruby attributes would just show as "dynamic_attr_label".
But another way to go might be to just use the attribute name and somehow make reference to the notion of force_update in the description for the attribute ala:
## # :attr_reader: dynamic_attr_label # # Description would somehow mention that there's a force_update=true|false # argument. sl_dynamic_attr :dynamic_attr_label do |param| # body end
A potential benefit here is that the attribute's rdoc output might look more "normal".
-
Include force_update on all such sl_dynamic_attr attributes? Although as @ju2wheels notes, the force_update argument works on all sl_dynamic_attr attributes, it is only mentioned in the description for the ones where there is a 5 minute auto-refresh implemented. We are thinking that we should mention force_update for all sl_dynamic_attr attributes. What do you think?
I'd like, if nothing else for completeness sake, to deal with this now so I'm hoping you can give me some thoughts here.
from softlayer-ruby.
Hmm... I thought I had added a comment but I don't see it now.
Please take my comments as a member of the community, not as a SoftLayer directive (or edict or whatever...)
1.
If we are to document these as attributes (i.e. with :attr_reader) then I prefer the "oddness" of the top option to overburdening the description for ALL of them.
However, I find myself second guessing using "attr" in the name to begin with. I mean what these really are are "getter" methods. The advantage of documenting them with :attr_reader is that they show up as attributes in rdoc, but they're not REALLY Ruby attributes. I'm kinda back and forth about whether documenting them as such is a good thing.
1.
If you're going to expose the force update parameter for the getter then it should probably be done everywhere.
The other way you might handle this is by finding some way to flag the "getter" routines as being sl_dynamic_attr
in the documentation then finding some where in the framework documentation to explain exactly what that means.
from softlayer-ruby.
-
How to doc the dynamic
sl_dynamic_attr
"attributes"?However, I find myself second guessing using "attr" in the name to begin with. I mean what these really are are "getter" methods. The advantage of documenting them with :attr_reader is that they show up as attributes in rdoc, but they're not REALLY Ruby attributes. I'm kinda back and forth about whether documenting them as such is a good thing.
I consulted with one of our more senior Ruby devs and his inclination was to doc the
sl_dynamic_attr
things as methods. I am coming around to this position as well. And as methods it's not odd or confusing at all to have the force_update argument. And most of these occasions are documented that way right now, modulo a few which I will fix.I think we should leave the
sl_attr
items as they are now, annotated as attributes via the :attr_reader: specifier.So can we agree to:
a. Documentsl_dynamic_attr
items as methods (as they mostly are now); and
b. Documentsl_attr
items as attributes (:attr_reader:) (as they also mostly are now)Sound OK?
-
force_update documented on all
sl_dynamic_attr
items?If you're going to expose the force update parameter for the getter then it should probably be done everywhere.
For sure I think the force_update parameter should be documented on the methods where there is the "auto update every 5 minutes" scheme, but I think it might already be in those cases. I will verify that.
@ju2wheels said pretty much the same thing that we should document the force_update parameter everywhere so let me make an update accordingly.
from softlayer-ruby.
For sure I think the force_update parameter should be documented on the methods where there is the "auto update every 5 minutes" scheme, but I think it might already be in those cases. I will verify that.
I dont think thats true for all of them. Its only in Account that there are dynamic attr entries that decide to update every 5 min if you dont specify force_update (why that was done I have no idea, but it seems to be one of the few oddball updates).
I think default behavior is update if force_update is true or should_update? is true.
from softlayer-ruby.
I think my pull request #98 is ready to go for this.
from softlayer-ruby.
OK. I will give the changes a quick look. With the expanded Object Filter capability I'll start thinking about a 3.1 release. Is there anything else you guys would like to get into that?
from softlayer-ruby.
Im going to have the cleanup stuff pushed today (need to add changes based on comments and the result limit options) and the base class for the first half of basic Network Monitors (making some last tweaks to implementation). Thats all I have right now thats ready to go.
from softlayer-ruby.
Related Issues (20)
- "provisioned" status not updating in wait_for_ready HOT 4
- NoMethodError: undefined method `capacity_restriction_maximum=' HOT 3
- Accessing ResourceMetadata without an API key HOT 2
- adding bandwidth allotment to VirtualServerOrder.rb HOT 2
- 500 error on getNextInvoiceTopLevelBillingItems for large accounts HOT 2
- VirtualServerOrder_Package class doesn't define VLANs HOT 4
- Bare Metal Package order examples not working HOT 1
- Exception: object_mask expects strings (ArgumentError) HOT 1
- VirtualServerUpgradeOrder cannot add additional disks HOT 2
- Tags still have referenceCount > 0 after all devices are canceled HOT 1
- BareMetal order fails for win 2008 HOT 4
- Placing order with upgrade item id 82 gave me the wrong result
- Baremetal order may fail for some datacenters
- Provide the ability to add datacentres by 'name' (currently its only 'id') for Image templates HOT 3
- Ruby code required for tagging Softlayer assets with TagKey and TagValue using SoftLayer API
- XMLRPC is now removed from stdlib
- Support for Family sizes HOT 1
- Request: release new version HOT 1
- Gem attributes are a mix of snake case and camel case - all should be snake case HOT 11
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from softlayer-ruby.