Skip to content

Conversation

@ofrobots
Copy link
Contributor

@ofrobotsofrobots commented Jun 2, 2016

Checklist
  • tests and code linting passes
  • the commit message follows commit guidelines
Affected core subsystem(s)

deps

Description of change

Pick the latest v8_inspector [1].

  • V8 5.1 compatibility
  • Modify parse builder templates to make coverity happy
  • The whitespace differences in the jinja2 sub-depenency do exist
    upstream. I am not sure how I missed them in the original import
    (ed2eacb).

[1] pavelfeldman/v8_inspector@3b56732

/cc @targos, @pavelfeldman, @bnoordhuis
EDIT: CI: https://ci.nodejs.org/job/node-test-pull-request/2909/

Pick the latest v8_inspector [1] with: * V8 5.1 compatibility * Modify parse builder templates to make coverity happy * The whitespace differences in the jinja2 sub-dependency do exist upstream. I am not sure how I missed them in the original import (ed2eac). [1] pavelfeldman/v8_inspector@3b56732
@nodejs-github-botnodejs-github-bot added the inspector Issues and PRs related to the V8 inspector protocol label Jun 2, 2016
@addaleax
Copy link
Member

LGTM

@MylesBorins
Copy link
Contributor

ci: https://ci.nodejs.org/job/node-test-pull-request/2910/

@ofrobots it would be awesome to get some basic tests in for the debugger. I'd be up for working on some to make the process of landing these updates clearer. Would we be able to draw on any of the v8 or chomium tests?

@cjihrig
Copy link
Contributor

LGTM

@jasnell
Copy link
Member

LGTM if CI is green

@MylesBorins
Copy link
Contributor

arm failures appear to be infra related LGTM

@addaleax
Copy link
Member

@ofrobots’ CI is green, fwiw.

DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
Copy link
Member

@bnoordhuisbnoordhuisJun 3, 2016

Choose a reason for hiding this comment

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

Can you undo the LF -> CRLF changes in this file?

EDIT: Or does this come verbatim from upstream?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

This did come verbatim from upstream. The difference shows up, as you guessed, because we land with --whitespace=fix

@bnoordhuis
Copy link
Member

LGTM. I usually land patches with git am --whitespace=fix. That would take care of most of the changes in this PR, I think.

@targos
Copy link
Member

LGTM

targos pushed a commit that referenced this pull request Jun 3, 2016
Pick the latest v8_inspector [1] with: * V8 5.1 compatibility * Modify parse builder templates to make coverity happy * The whitespace differences in the jinja2 sub-dependency do exist upstream. I am not sure how I missed them in the original import (ed2eac). [1] pavelfeldman/v8_inspector@3b56732 PR-URL: #7118 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
@targos
Copy link
Member

Landed in 8847777

@targostargos closed this Jun 3, 2016
@MylesBorinsMylesBorins added this to the 7.0.0 milestone Jun 14, 2016
Fishrock123 pushed a commit that referenced this pull request Jul 5, 2016
Pick the latest v8_inspector [1] with: * V8 5.1 compatibility * Modify parse builder templates to make coverity happy * The whitespace differences in the jinja2 sub-dependency do exist upstream. I am not sure how I missed them in the original import (ed2eac). [1] pavelfeldman/v8_inspector@3b56732 PR-URL: #7118 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
@Fishrock123Fishrock123 mentioned this pull request Jul 5, 2016
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

inspectorIssues and PRs related to the V8 inspector protocol

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants

@ofrobots@addaleax@MylesBorins@cjihrig@jasnell@bnoordhuis@targos@Fishrock123@nodejs-github-bot