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

Stream files during code transform #585

Merged
merged 14 commits into from Jan 15, 2019

Conversation

Projects
None yet
4 participants
@olvesh
Copy link
Contributor

commented Nov 28, 2018

No description provided.

@CLAassistant

This comment has been minimized.

Copy link

commented Nov 28, 2018

CLA assistant check
All committers have signed the CLA.

@olvesh olvesh changed the title WIP: Stream files during code transform. Should work, but doesnt :-( Stream files during code transform Nov 28, 2018

@ddgenome

This comment has been minimized.

Copy link
Member

commented Nov 28, 2018

@olvesh so do you think this PR is working now and ready to review/merge?

@ddgenome ddgenome self-assigned this Nov 28, 2018

@olvesh

This comment has been minimized.

Copy link
Contributor Author

commented Nov 28, 2018

Yeah, it works at least. Haven’t got to use the too much yet, so maybe it should have some additions…
Not completely sure about the recipient path thingy makes sense in this setting.

@ddgenome
Copy link
Member

left a comment

Thank you for the contribution! I think there are a few issues that need addressed, although I am not an expert on the Node.js stream interface.

Show resolved Hide resolved lib/api-helper/project/fileCopy.ts Outdated
Show resolved Hide resolved lib/api-helper/project/fileCopy.ts
Show resolved Hide resolved lib/api-helper/project/fileCopy.ts Outdated
Show resolved Hide resolved lib/api-helper/project/fileCopy.ts Outdated
Show resolved Hide resolved lib/api-helper/project/fileCopy.ts Outdated
Show resolved Hide resolved test/api-helper/project/fileCopy.test.ts Outdated
Show resolved Hide resolved test/api-helper/project/fileCopy.test.ts Outdated
Show resolved Hide resolved lib/api-helper/project/fileCopy.ts Outdated
@ddgenome
Copy link
Member

left a comment

More feedback.

Show resolved Hide resolved lib/api-helper/project/fileCopy.ts Outdated
Show resolved Hide resolved lib/api-helper/project/fileCopy.ts

olvesh added some commits Dec 4, 2018

Merge branch 'master' into code-transform-stream-files
* master: (104 commits)
  Autofix: tslint
  Autofix: TypeScript header
  Fix misplaced comment
  Changelog: add release 1.2.0
  Version: increment after 1.2.0 release
  Autofix: Third party licenses
  Update client to 1.2.0
  Autofix: Third party licenses
  Update all dependencies
  Changelog: #639 to fixed
  Correct typo
  Changelog: #642 to added
  Auto merge pull request #642 from atomist/sdm
  Changelog: 7fd6fab to fixed
  Fix issues when pushTest is missing on project listener
  Update doc on goal execution listener
  Autofix: Third party licenses
  Changelog: 87a5514 to changed
  Changelog: 1a3e3e2 to changed
  Delint
  ...
@ddgenome
Copy link
Member

left a comment

Sorry for the delay on this, the dates on the comments and pushes led me to think it wasn't updated. Thanks for the changes, I think it is about ready. Just a couple minor things.

Show resolved Hide resolved lib/api-helper/project/fileCopy.ts Outdated
Show resolved Hide resolved lib/api-helper/project/fileCopy.ts Outdated
Show resolved Hide resolved test/api-helper/project/fileCopy.test.ts Outdated
Show resolved Hide resolved test/api-helper/project/fileCopy.test.ts Outdated
Show resolved Hide resolved test/api-helper/project/fileCopy.test.ts Outdated

olvesh added some commits Jan 11, 2019

@ddgenome
Copy link
Member

left a comment

LGTM, thanks for your patience and perseverance!

@ddgenome ddgenome merged commit ac2f7f8 into atomist:master Jan 15, 2019

1 check passed

license/cla Contributor License Agreement is signed.
Details

atomist-bot added a commit that referenced this pull request Jan 15, 2019

Changelog: #585 to added
[atomist:generated]
@olvesh

This comment has been minimized.

Copy link
Contributor Author

commented Jan 16, 2019

Thanks for guiding a novice TypeScripter through his first pull request (for TS)!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.