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

Support definitions that don't have tags. Closes #140 #144

Merged

Conversation

brendo
Copy link
Contributor

@brendo brendo commented Jun 27, 2017

Note I've pulled in an example from the OAI project which requires more tickets before we can actually display it in full. For now, it's just used as an example of a definition without tags.

Notes:

  • sortFunc only applies to definitions with tags. It just doesn't make much sense otherwise.
  • minor bumps to eslint packages (no more warnings when installing)
  • start to extract logic out of the one parser by creating a navigationParser file. It's a start, but honestly I'm not super worried about optimising too much in this space until we pull this out as a separate package.

Example:

http://localhost:8200/?url=https://raw.githubusercontent.com/OAI/OpenAPI-Specification/OpenAPI.next/examples/v3.0/api-with-examples.yaml


for (let i = 0; i < tags.length; i++) {
for (let i = 0, tagLength = tags.length; i < tagLength; i++) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I just want to add, it would be nice to integrate benchmarks in our tests, for regressions in the parser (or not-regressions!).

Below is an example benchmark test:

export function benchmark (timeout, fn) {
  const timeoutAt = Date.now() + timeout
  let iterations = 0

  do {
    fn()
    ++iterations
  } while (Date.now() <= timeoutAt)

  return iterations
}

it('benchmark', () => {
  const base = 2000 // Keep updating this manually, but for CI?
  // In future you'd want to import the previous stable version as a dev dep (maybe git commit hash?) and test against that

  const current = benchmark(500, () => {
    doSomeParsing('foo')
  })

  const percentDiff = Math.round(((current - base) / base) * 100)

  console.dir(`doSomeParsing: ${base} -> ${current} (%${percentDiff})`)

  expect(percentDiff).toBeGreaterThan(20) // It must be 20 percent faster
}

In theory iterations will scale across systems, but the baseline will be awkward.

Copy link
Contributor Author

@brendo brendo Jun 27, 2017

Choose a reason for hiding this comment

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

Good idea, we should track this as a separate ticket. I raised #146

@brendo brendo force-pushed the bugfix-140-support-definitions-without-tags branch from bae1e73 to f029b6c Compare June 28, 2017 00:46
@brendo brendo force-pushed the bugfix-140-support-definitions-without-tags branch from f029b6c to 4ec9eb7 Compare June 28, 2017 06:14
Copy link
Collaborator

@nfour nfour left a comment

Choose a reason for hiding this comment

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

👌

@nfour nfour merged commit 583b8d9 into temando:master Jun 28, 2017
@brendo brendo deleted the bugfix-140-support-definitions-without-tags branch June 28, 2017 06:24
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.

2 participants