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

Fix initiator and update package lock #9

Merged
merged 4 commits into from
Mar 14, 2018

Conversation

ddadon10
Copy link

Hello,

I fixed the initiator, its now have different properties according to its type property:

For all type:

  • detail (serialized version of the whole initiator object)
  • type (script, parser or other)

Parser type:

  • url
  • line number

Script type:

  • url of the top element in the call frame
  • line number of the top element in the call frame
  • column number of the top element in the call frame
  • function's name of the top element in the call frame
  • script's id of the top element in the call frame

And I updated the package.lock.json

Thanks 😄

@ddadon10
Copy link
Author

Strange, npm run lint do not raise me any error on my machine, do I miss something ?

@ddadon10
Copy link
Author

Also, I found that the CI has failed with the precedent commit https://travis-ci.org/sitespeedio/chrome-har/jobs/322701738 too.

Do I have to change something in the code or does the CI criteria (or the linter in the code) have to be change ?

Thanks 😄

index.js Outdated
const topCallFrame = params.initiator.stack.callFrames[0];
entry._initiator = topCallFrame.url;
entry._initiator_line = topCallFrame.lineNumber + 1; // Because lineNumber is 0 based
entry._initiator_column = topCallFrame.columnNumber;
Copy link
Member

Choose a reason for hiding this comment

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

This is also 0-based, right?

Copy link
Author

Choose a reason for hiding this comment

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

Idk, will check 😄

Copy link
Author

Choose a reason for hiding this comment

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

@tobli
Copy link
Member

tobli commented Mar 14, 2018

Since we don’t lock down dev dependencies, the version of prettier that’s the current at the time will be run. There are sometimes small formatting changes to prettier, so a previously green build might shift to red, just by running npm install at a later time. Not ideal, we should probably lock it down, at least to a major version.

I can have a closer look, hopefully later tonight. Changes look awesome btw. =)

@tobli tobli merged commit 3e60417 into sitespeedio:master Mar 14, 2018
@ddadon10 ddadon10 mentioned this pull request Mar 15, 2018
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