Skip to content

Conversation

@annxingyuan
Copy link
Contributor

@annxingyuan annxingyuan commented Mar 9, 2020

To see the logs from the Cloud Build CI, please join either our discussion or announcement mailing list.


This change is Reviewable

@annxingyuan annxingyuan self-assigned this Mar 9, 2020
Copy link
Contributor

@tafsiri tafsiri left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 1 approvals obtained (waiting on @annxingyuan, @dsmilkov, @nsthorat, and @tafsiri)


tfjs-data/package.json, line 56 at r1 (raw file):

  },
  "peerDependencies": {
    "@tensorflow/tfjs-core": "link:../tfjs-core",

Wondering about this change from link: to a particular version. Having a particular version looks correct to me, but I wonder how it got changed to "link". Could this be related to @lina128 recent infra changes?

Copy link
Contributor

@nsthorat nsthorat left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 3 files at r1.
Reviewable status: :shipit: complete! 1 of 1 approvals obtained (waiting on @dsmilkov, @lina128, and @nsthorat)


tfjs-data/package.json, line 56 at r1 (raw file):

Previously, tafsiri (Yannick Assogba) wrote…

Wondering about this change from link: to a particular version. Having a particular version looks correct to me, but I wonder how it got changed to "link". Could this be related to @lina128 recent infra changes?

yarn link-core-master needs to be added for data: https://github.com/tensorflow/tfjs/pull/2842/files

we can do this in a follow up

@annxingyuan annxingyuan merged commit de7219c into master Mar 9, 2020
@annxingyuan annxingyuan deleted the b1.6.1 branch March 9, 2020 15:25
@lina128
Copy link
Collaborator

lina128 commented Mar 9, 2020

Reviewable status: 0 of 1 approvals obtained (waiting on @annxingyuan, @dsmilkov, @nsthorat, and @tafsiri)

tfjs-data/package.json, line 56 at r1 (raw file):

  },
  "peerDependencies": {
    "@tensorflow/tfjs-core": "link:../tfjs-core",

Wondering about this change from link: to a particular version. Having a particular version looks correct to me, but I wonder how it got changed to "link". Could this be related to @lina128 recent infra changes?

I think this is because layers yarn build will link core now, ci should change to use layers' yarn build-ci, which doesn't do the link. Submitting a PR now.

@lina128
Copy link
Collaborator

lina128 commented Mar 9, 2020

Reviewable status: 0 of 1 approvals obtained (waiting on @annxingyuan, @dsmilkov, @nsthorat, and @tafsiri)

tfjs-data/package.json, line 56 at r1 (raw file):

  },
  "peerDependencies": {
    "@tensorflow/tfjs-core": "link:../tfjs-core",

Wondering about this change from link: to a particular version. Having a particular version looks correct to me, but I wonder how it got changed to "link". Could this be related to @lina128 recent infra changes?

I think this is because layers yarn build will link core now, ci should change to use layers' yarn build-ci, which doesn't do the link. Submitting a PR now.

Take my word back, above is another issue, yes, you are right, the link you are seeing here is related to recent infra change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants