Skip to content

Conversation

@2Cubed
Copy link
Member

@2Cubed 2Cubed commented Aug 19, 2017

  • Message.Text can not be filled, as described in the FIXME comment
    (schemas/schemas.go). This is critical to fix!
  • Packets other than Message are not currently supported, as I was not
    able to figure out how to make that work with Go's type system. This
    would be a nice feature to have, though, so custom commands could
    potentially return other packets (for example, triggering a ban after a
    custom warning or something.)
  • util/jsonapi.go was changed to allow for the standard placement of
    JSON-API tags (e.g. "attr,foo" instead of "foo,attr"). Is this an issue,
    @RPiAwesomeness?
  • createdAt and id are not being populated, but I suspect this has
    simply not been completed yet.

 - Message.Text can not be filled, as described in the FIXME comment
(schemas/schemas.go). This is critical to fix!
 - Packets other than Message are not currently supported, as I was not
able to figure out how to make that work with Go's type system. This
would be a nice feature to have, though, so custom commands could
potentially return other packets (for example, triggering a ban after a
custom warning or something.)
 - util/jsonapi.go was changed to allow for the standard placement of
JSON-API tags (e.g. "attr,foo" instead of "foo,attr"). Is this an issue,
@RPiAwesomeness?
 - createdAt and id are not being populated, but I suspect this has
simply not been completed yet.
)

type Message struct {
Text []Component `json:"text"` // FIXME: Anything besides [] breaks this. It is assumed that this is due to the interface being used for Component.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RPiAwesomeness This is critical to fix before merging; we obviously need to be able to PATCH new commands! Any ideas?

default: // Ignore any other tags
}
// Need to set the keys w/ their names here if it's a struct
switch split[0] {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RPiAwesomeness Will this change have any negative side effects?

Role int `jsonapi:"role,attr" validate:"gte=0,lte=256"`
Target string `jsonapi:"target,attr"`
User string `jsonapi:"user,attr"`
ID string `json:"id" validate:"-"`
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None of these fields seem to be automatically populated. (Instead, it looks like they're just "made up" later on.) I'm assuming this is still WIP?

Not ready for production; see HACK/FIXME/TODO comments.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants