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

Sign Apex Plugin (@W-7566604) #43

Merged
merged 14 commits into from
Aug 12, 2020
Merged

Sign Apex Plugin (@W-7566604) #43

merged 14 commits into from
Aug 12, 2020

Conversation

AnanyaJha
Copy link
Contributor

@AnanyaJha AnanyaJha commented Aug 11, 2020

What does this PR do?

This PR takes care of signing the apex plugin. Uses the latest version of the orb.

What issues does this PR fix or reference?

@W-7566604@

@AnanyaJha AnanyaJha requested a review from a team as a code owner August 11, 2020 01:22
@AnanyaJha AnanyaJha requested review from xyc, sfsholden and lcampos August 11, 2020 01:23
@AnanyaJha AnanyaJha requested a review from xyc August 11, 2020 03:47
@AnanyaJha
Copy link
Contributor Author

AnanyaJha commented Aug 12, 2020

re: standup
let me know if you have any feedback on this config, hoping to get this merged & do a test release today so we can switch to just using the scripts if necessary

Copy link
Contributor

@xyc xyc left a comment

Choose a reason for hiding this comment

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

Looks good. Let's get this merged and test it out

@@ -120,6 +137,19 @@ jobs:
export RELEASE_TAG="$(node -pe "require('./packages/apex-node/package.json').version")"
git tag v${RELEASE_TAG}
git push --tags
publish-plugin-apex:
Copy link
Contributor

Choose a reason for hiding this comment

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

One thing that's not good with this approach is that every publish job independently bumps the package version. This will cause the package versions to be out of sync every time we publish. We should bump the version once and then kick off publishing the packages.

Copy link
Contributor

Choose a reason for hiding this comment

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

Are we going to keep version number in apex-node the same as plugin? I think it has been 0.0.x

Copy link
Contributor

Choose a reason for hiding this comment

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

which is different from what we use in templates (49.x.x), but I think version sync still makes sense

Copy link
Contributor Author

@AnanyaJha AnanyaJha Aug 12, 2020

Choose a reason for hiding this comment

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

since apex-node is already a few versions ahead of the plugin, would we just bump the plugin's version number up to 0.0.5? (or whatever the current version of apex-node is)

Copy link
Contributor

Choose a reason for hiding this comment

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

When we bump the versions, I think we'd also need to update the version of the library dependency in the plugin @AnanyaJha

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah make senses to me to sync with whatever the current version of apex-node is. Let's bump apex-node and find out the version, then sync plugin-apex to that version (npm version).

Copy link
Contributor

Choose a reason for hiding this comment

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

Lerna takes care of updating all the references needed, you just need to run the version update part through lerna

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, we discussed that offline and it's been taken care of by lerna (yarn bump-versions).

Copy link
Contributor

Choose a reason for hiding this comment

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

Also currently library & plugin version are in sync so we don't need to make any changes

- run-win-tests
- node-latest
- node-12
- node-10
Copy link
Contributor

Choose a reason for hiding this comment

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

@AnanyaJha After rebasing, this is the change that Xiaoyi and I want to add.

Suggested change
- node-10
- node-10
- hold

Copy link
Contributor

Choose a reason for hiding this comment

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

Note that we'll also need to add prepublish and hold to this requires list for publish-plugin-apex as well.

package.json Outdated
@@ -34,5 +34,6 @@
},
"publishConfig": {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can also remove publishConfig here

publish-apex-node:
<<: *defaults
steps:
- set-npmrc
Copy link
Contributor

Choose a reason for hiding this comment

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

can remove set-npmrc here too.

Copy link
Contributor

Choose a reason for hiding this comment

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

@AnanyaJha AnanyaJha merged commit ff8e174 into develop Aug 12, 2020
@AnanyaJha AnanyaJha deleted the aj/signPlugin branch August 12, 2020 21:18
sfsholden pushed a commit that referenced this pull request Aug 12, 2020
@sfsholden sfsholden changed the title Sign Apex Plugin Sign Apex Plugin (@W-7566604) Aug 13, 2020
sfsholden pushed a commit that referenced this pull request Aug 20, 2020
sfsholden pushed a commit that referenced this pull request Aug 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants