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

Remove annotation code #24

Open
kylef opened this issue Jul 5, 2017 · 7 comments
Open

Remove annotation code #24

kylef opened this issue Jul 5, 2017 · 7 comments
Milestone

Comments

@kylef
Copy link
Member

kylef commented Jul 5, 2017

Annotation Code's are difficult to use for any meaning as they are not universally unique. One parser can treat one code with some meaning and another parser could treat the exact same code with another meaning.

This is already the case with the Fury Swagger adapter and API Blueprint parser. To the Swagger parser, code 3 is for features that are not supported (https://github.com/apiaryio/fury-adapter-swagger/blob/master/src/annotations.js#L17) and in Drafter code 3 is used for other validations (https://github.com/apiaryio/drafter/blob/4b5ae151e0a40f4a53bf07a86b4ea8fabb70ca7e/test/fixtures/parse-result/error-warning.json#L90). These code numbers provide real no benefit to consumers.

I believe that profile or origin links are better suited for a consumer to understand the semantics behind an annotation. Origin links are already available in the Swagger parser but are missing from Drafter API Blueprint parser. These links provide a reliable way for a consumer to understand the semantics behind an annotation, it also provides the consumer a way to follow a link for further information.

{
  "element": "annotation",
  "attributes": {
    "code": {
      "element": "number",
      "content": 5
    }
  },
  "content": "something was wrong"
}

vs

{
  "element": "annotation",
  "meta": {
    "links": [
      {
        "element": "link",
        "attributes": {
          "relation": { "element": "string", "content": "profile" },
          "href": { "element": "string", "content": "https://apielements.org/1/warnings/validation" },
        }
      }
    ]
  },
  "content": "something was wrong"
}

Would love to hear your opinions @smizell / @w-vi.

@w-vi
Copy link
Member

w-vi commented Jul 6, 2017

I am definitely up for it, I find the links being very useful because not always the error message is clear enough and further info is needed. although I think we should make an attempt to unify the codes as well. I think that a general categorization of the warnings/errors is useful for tools using the parse result.

@kylef
Copy link
Member Author

kylef commented Jul 6, 2017

although I think we should make an attempt to unify the codes as well

@w-vi Would we still need to provide error codes if the profile/origin link can replace their purpose? I was proposing we remove the code completely.

@smizell
Copy link
Contributor

smizell commented Jul 6, 2017

I was personally not a fan of the codes, and I agree that the links provide the same functionality with a lot more flexibility. So I think you can capture the same info and more with links.

I think the part that will take some work is keeping the links pointing to the correct places. This is because some annotations will be parser specific rather than related to API Elements itself. But the profile link example you have above is a good one where you can fit generic errors/warnings/info into the API Elements spec itself that may be reused by tooling.

@kylef kylef modified the milestone: 1.0 Jul 6, 2017
@w-vi
Copy link
Member

w-vi commented Jul 7, 2017

Codes are convenient for programmatic approach, sure I can match the strings of the link to distinguish between warning error etc but then I need to know the errors/warnings beforehand as from the new annotation I have no idea what is it as in how I know that this is an error and not just some informational warning. To clarify, we should use codes more for categories of errors/warnings and not as the code being a sole identifier of an error.

@smizell
Copy link
Contributor

smizell commented Jul 7, 2017

Seems like it might work well to use classes instead of codes for those generic purposes.

@kylef
Copy link
Member Author

kylef commented Jul 7, 2017

Seems like it might work well to use classes instead of codes for those generic purposes.

That sounds good to me.

I was going by the example in Swagger adapter where the origin links have a 1-1 mapping between codes for all URLs except one (https://github.com/apiaryio/fury-adapter-swagger/blob/master/src/annotations.js#L17) the fragment is placed into the URL.

@w-vi
Copy link
Member

w-vi commented Jul 7, 2017

classes are good too. My main point is that I'd like to keep some generic way how tools can handle annotations and that those classes or codes or whatever should be unified in all parsers.

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

No branches or pull requests

3 participants