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

src: remove unnecessary forward declaration #13081

Conversation

danbev
Copy link
Contributor

@danbev danbev commented May 17, 2017

I can't see that the forward declaration of class Connection is needed
and wanted to raise this in case it was overlooked after a previous
change.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

src

I can't see that the forward declaration of class Connection is needed
and wanted to raise this in case it was overlooked after a previous
change.
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem. labels May 17, 2017
@danbev
Copy link
Contributor Author

danbev commented May 17, 2017

@danbev
Copy link
Contributor Author

danbev commented May 22, 2017

test/arm-fanned failure does not look related

console output:

Reinitialized existing Git repository in /home/iojs/build/workspace/node-test-binary-arm/.git/
+ git fetch --no-tags file:///home/iojs/.ccache/node.shared.reference +refs/heads/master:refs/remotes/reference/master +refs/heads/v4.x-staging:refs/remotes/reference/v4.x-staging +refs/heads/v6.x-staging:refs/remotes/reference/v6.x-staging +refs/heads/v7.x-staging:refs/remotes/reference/v7.x-staging +refs/heads/v8.x-staging:refs/remotes/reference/v8.x-staging
From file:///home/iojs/.ccache/node.shared.reference
   5debcce..6342988  master     -> reference/master

real	0m2.386s
user	0m0.470s
sys	0m0.390s
+ git fetch --no-tags file:///home/iojs/.ccache/node.shared.reference +refs/heads/jenkins-node-test-commit-arm-fanned-8800-binary-pi1p/cc-armv7:refs/remotes/jenkins_tmp
fatal: Couldn't find remote ref refs/heads/jenkins-node-test-commit-arm-fanned-8800-binary-pi1p/cc-armv7
fatal: The remote end hung up unexpectedly

real	0m0.303s
user	0m0.090s
sys	0m0.030s
+ echo Could not fetch refs/heads/jenkins-node-test-commit-arm-fanned-8800-binary-pi1p/cc-armv7 from the local reference repo, trying GitHub.
Could not fetch refs/heads/jenkins-node-test-commit-arm-fanned-8800-binary-pi1p/cc-armv7 from the local reference repo, trying GitHub.
+ grep -q '^github.com' /home/iojs/.ssh/known_hosts
+ ssh-agent sh -c 'ssh-add **** && git fetch --no-tags [email protected]:janeasystems/node_binary_tmp.git +refs/heads/jenkins-node-test-commit-arm-fanned-8800-binary-pi1p/cc-armv7:refs/remotes/jenkins_tmp'
Identity added: **** (rsa w/o comment)
fatal: Couldn't find remote ref refs/heads/jenkins-node-test-commit-arm-fanned-8800-binary-pi1p/cc-armv7
Build step 'Execute shell' marked build as failure
TAP Reports Processing: START
Looking for TAP results report in workspace using pattern: *.tap
Saving reports...
Processing '/var/lib/jenkins/jobs/node-test-binary-arm/configurations/axis-RUN_SUBSET/1/axis-label/pi3-raspbian-jessie/builds/8091/tap-master-files/test.tap'
Parsing TAP test result [/var/lib/jenkins/jobs/node-test-binary-arm/configurations/axis-RUN_SUBSET/1/axis-label/pi3-raspbian-jessie/builds/8091/tap-master-files/test.tap].
TAP Reports Processing: FINISH
Checking ^not ok
Notifying upstream projects of job completion
Finished: FAILURE

danbev added a commit to danbev/node that referenced this pull request May 22, 2017
I can't see that the forward declaration of class Connection is needed
and wanted to raise this in case it was overlooked after a previous
change.

PR-URL: nodejs#13081
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@danbev
Copy link
Contributor Author

danbev commented May 22, 2017

Landed in d54ec72

@danbev danbev closed this May 22, 2017
@danbev danbev deleted the node_crypto.h-unnecessary-forward-declaration branch May 22, 2017 05:46
jasnell pushed a commit that referenced this pull request May 23, 2017
I can't see that the forward declaration of class Connection is needed
and wanted to raise this in case it was overlooked after a previous
change.

PR-URL: #13081
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
jasnell pushed a commit that referenced this pull request May 23, 2017
I can't see that the forward declaration of class Connection is needed
and wanted to raise this in case it was overlooked after a previous
change.

PR-URL: #13081
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@jasnell jasnell mentioned this pull request May 28, 2017
@gibfahn gibfahn mentioned this pull request Jun 15, 2017
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants