Skip to content
This repository has been archived by the owner on Jun 18, 2021. It is now read-only.

Fixes/port for node-report on z/OS: #129

Merged
merged 4 commits into from
Jun 6, 2019
Merged

Conversation

gabylb
Copy link
Contributor

@gabylb gabylb commented May 13, 2019

  • Fix compiler errors
  • Change ebcdic output to ascii
  • Native Stack Trace and JavaScript Stack Trace
  • Command line
  • Loaded libraries
  • Hostname
  • Filename output in fs_event
  • Listening socket tcp uv handle
  • OS Version check

- Fix compiler errors
- Change ebcdic output to ascii
- Native Stack Trace and JavaScript Stack Trace
- Command line
- Loaded libraries
- Hostname
- Filename output in fs_event
- Listening socket tcp uv handle
- OS Version check
@richardlau
Copy link
Member

cc @nodejs/platform-s390

Copy link
Member

@richardlau richardlau left a comment

Choose a reason for hiding this comment

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

Generally looks good. Left a couple of comments.

binding.gyp Outdated Show resolved Hide resolved
src/node_report.cc Outdated Show resolved Hide resolved
@richardlau
Copy link
Member

I've been able to verify that these changes work with an internal IBM build of https://github.com/ibmruntimes/node/tree/v8.16.0.zos. They don't work with the current publicly available Node.js 8 on z/OS public beta (based on v8.15.1).

Which leads to the question of maintaining z/OS support if we land this. While we do have z/OS machines in the community CI (used for building libuv) I don't really want to have to build Node.js from source (as it will add a lot of time to the CI runs). So it looks like we'll be reliant on the z/OS folks at IBM to keep an eye on the module and update as necessary.

cc @nodejs/platform-s390 @mhdawson @gabylb @IgorTodorovskiIBM

CI runs to show these changes don't break existing platform support:
Node.js 8: https://ci.nodejs.org/view/post-mortem/job/nodereport-continuous-integration/316/ (✔️)
Node.js 10: https://ci.nodejs.org/view/post-mortem/job/nodereport-continuous-integration/314/ (✔️)
Node.js 12: https://ci.nodejs.org/view/post-mortem/job/nodereport-continuous-integration/315/ (✔️)

@mhdawson
Copy link
Member

@richardlau would it be possible to setup a CI job in the community which pulls the Node.js binaries that @IgorTodorovskiIBM team's provides and then run the node-report tests. We might even be able to have his team/the IBM Node.js add those binaries to the z/OS machines when they are released to make it even easier?

@mhdawson
Copy link
Member

mhdawson commented Jun 4, 2019

@IgorTodorovskiIBM, @joransiu any thoughts on my last comment?

@joransiu
Copy link

joransiu commented Jun 4, 2019

@mhdawson: I think that's a good idea.

@richardlau richardlau merged commit 22ef3b2 into nodejs:master Jun 6, 2019
richardlau added a commit that referenced this pull request Jun 6, 2019
 * Add support for z/OS (#129) (Gaby Baghdadi)
@gabylb gabylb deleted the zos branch October 28, 2019 15:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants