GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Convert reports to JSON by oliversalzburg · Pull Request #135 · stacktracejs/stacktrace.js

Conversation

@oliversalzburg

@oliversalzburg oliversalzburg commented Oct 15, 2015

Copy link
Copy Markdown
Contributor

Includes JSON3 in the polyfill version
Fixes #133

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
houndci-bot
houndci-bot reviewed Oct 15, 2015
View reviewed changes
Comment thread dist/stacktrace.min.js Outdated

@houndci-bot houndci-bot Oct 15, 2015

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Line is too long.
Missing semicolon.
Expected an identifier and instead saw '=='.
Expected an operator and instead saw 'typeof'.
Expected an assignment or function call and instead saw an expression.
Wrap an immediate function invocation in parens to assist the reader in understanding that the expression is the result of a function, and not the function itself.
'e' is already defined.
't' is already defined.
Possible strict violation.
Expected '{' and instead saw 'throw'.
Expected '{' and instead saw 'return'.
'i' is already defined.
Use '!==' to compare with 'null'.
Too many errors. (50% scanned).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@oliversalzburg oliversalzburg mentioned this pull request Oct 15, 2015
Problem in sending error report using Stacktrace.report #133
Closed
@oliversalzburg

oliversalzburg commented Oct 15, 2015

Copy link
Copy Markdown
Contributor Author

@eriwen Can we exclude dist from HoundCI checks? Also, do you know what the problem is with Travis?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@eriwen

eriwen commented Oct 16, 2015

Copy link
Copy Markdown
Member

@oliversalzburg Looks like we can exclude files from Hound by adding a .jshintignore file with the following content:

dist/*

I'm not sure about Travis. It works on master and we haven't made changes. Maybe something is wrong with the PR-specific config somehow...
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@eriwen

eriwen commented Oct 16, 2015

Copy link
Copy Markdown
Member

Also, this looks good except for the one thing; thank you!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
eriwen
eriwen reviewed Oct 16, 2015
View reviewed changes
Comment thread gulpfile.js Outdated

@eriwen eriwen Oct 16, 2015

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This should go in polyfills

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@oliversalzburg

oliversalzburg commented Oct 16, 2015

Copy link
Copy Markdown
Contributor Author
  • Fixed the gulp build (wow, what a mistake)
  • Added .jshintignore
  • Fixed the test
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@oliversalzburg
Convert reports to JSON
269ea34 
Includes JSON3 in the polyfill version
Fixes #133
houndci-bot
houndci-bot reviewed Oct 16, 2015
View reviewed changes
Comment thread spec/stacktrace-spec.js

@houndci-bot houndci-bot Oct 16, 2015

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Line is too long.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@oliversalzburg oliversalzburg Oct 16, 2015

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

How can that still be too long? O_o

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@eriwen eriwen Oct 17, 2015

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Hm.. I'm guessing the default line length limit is 80.

Sorry about the annoying Hound stuff; don't worry about this kinda thing. I may just disable it since JSHint is part of the build anyway.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@eriwen

eriwen commented Oct 17, 2015

Copy link
Copy Markdown
Member

Looks good! :shipit:
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
oliversalzburg added a commit that referenced this pull request Oct 18, 2015
@oliversalzburg
Merge pull request #135 from oliversalzburg/fix/report
2053cd7 
Convert reports to JSON
@oliversalzburg
oliversalzburg merged commit 2053cd7 into stacktracejs:master Oct 18, 2015
@oliversalzburg
oliversalzburg deleted the fix/report branch October 18, 2015 14:01
eriwen pushed a commit that referenced this pull request Apr 16, 2016
@oliversalzburg
Merge pull request #135 from oliversalzburg/fix/report
fba3c04 
Convert reports to JSON
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@oliversalzburg @eriwen @houndci-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.