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

WIP: start on #23 #25

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kingdonb
Copy link

This is a start on addressing #23 which is to catch up this file with the attributes in the beeminder api reference.

I did some reordering so that it would be clearer at-a-glance that the list is the same as the one in Goal Resource

I noticed that "panic", "user", and possibly some others are not in the list under goal reference. I wonder if this is an omission from the API reference, or if there's something else going on here.

User I could understand if it was omitted because it is part of the RESTful route, so maybe it makes sense to remain in the list but shouldn't be counted? But panic seems like an attribute that is in use in several places in the gem, it seems like it probably hasn't been removed.

This is a start on addressing beeminder#23 which is to catch up this file with
the attributes in the beeminder api reference
@kingdonb
Copy link
Author

I'm going to add the docu-strings and look at each new attr, then try to decide if it makes sense to be attr_reader or attr_accessor. Some of them are probably read-only, and others are user-configurable. If there are any "surprise, this one can't be written through the API" surprises in there that I should be careful of, please let me know!

There is probably also testing around these attrs ? I haven't looked deeply enough at the change to see for sure, but I'll follow all of this up before I remove the "WIP" tag from the title.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant