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

refactor!: upgrade octokit/gts/typescript #381

Merged
merged 5 commits into from
Mar 27, 2020
Merged

refactor!: upgrade octokit/gts/typescript #381

merged 5 commits into from
Mar 27, 2020

Conversation

bcoe
Copy link
Contributor

@bcoe bcoe commented Mar 27, 2020

@xiaozhenliu-gg5 I ended up pulling over your work from #380 into this PR, as I had to do a significant amount of refactoring of types to upgrade @octokit (I'd been putting off this update for weeks because I knew it would take so much playing around).

Sorry to have wasted a little bit of your time on #380.

bcoe added 2 commits March 26, 2020 18:01
BREAKING CHANGE: octokit has significant interface changes.
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 27, 2020
@bcoe bcoe requested a review from chingor13 March 27, 2020 01:29
@codecov
Copy link

codecov bot commented Mar 27, 2020

Codecov Report

Merging #381 into master will decrease coverage by 8.04%.
The diff coverage is 92.1%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #381      +/-   ##
==========================================
- Coverage   88.95%   80.91%   -8.05%     
==========================================
  Files          27       13      -14     
  Lines        3106     2305     -801     
  Branches      292      167     -125     
==========================================
- Hits         2763     1865     -898     
- Misses        342      439      +97     
  Partials        1        1
Impacted Files Coverage Δ
src/releasers/php-yoshi.ts 91.01% <100%> (-2.35%) ⬇️
src/updaters/root-composer.ts 86.2% <100%> (+0.24%) ⬆️
src/updaters/changelog.ts 89.28% <100%> (-10.72%) ⬇️
src/graphql-to-commits.ts 88.46% <100%> (-1.65%) ⬇️
src/updaters/php-client-version.ts 88.37% <100%> (ø) ⬆️
src/updaters/version.ts 100% <100%> (ø) ⬆️
src/github-release.ts 95.2% <100%> (-2.39%) ⬇️
src/commit-split.ts 100% <100%> (ø) ⬆️
src/updaters/php-manifest.ts 88.73% <100%> (ø) ⬆️
src/github.ts 65.54% <83.33%> (-8.82%) ⬇️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cec602e...47a43fd. Read the comment docs.

@xiaozhenliu-gg5
Copy link

It's ok, thanks for doing this! @bcoe @octokit bites me for linter a lot so I have to disable some no-var-requires check for it. You are doing this in a cleaner way!
Next time I will let you know which API I'm working on so that our work won't be overlapped😀

"@types/chai": "^4.1.7",
"@types/mocha": "^7.0.0",
"@types/node": "^11.13.6",
"@types/semver": "^7.0.0",
"c8": "^7.0.0",
"chai": "^4.2.0",
"cross-env": "^7.0.0",
"gts": "^1.0.0",
"gts": "^2.0.0-alpha.8",
Copy link
Contributor

Choose a reason for hiding this comment

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

I saw some of package use next tag here. Dose it matter here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I prefer being explicit with the dependency, as soon as we do a few GAPIC library updates, I think we can go ahead and migrate gts to 2.x 😄

@@ -11,14 +11,15 @@
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
/* eslint-disable @typescript-eslint/no-var-requires */
Copy link
Contributor

Choose a reason for hiding this comment

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

eslint-disable affect entire file, is this what we want?

@bcoe bcoe merged commit 4e0bf11 into master Mar 27, 2020
@bcoe bcoe deleted the update-typescript branch March 27, 2020 19:19
@release-please release-please bot mentioned this pull request Mar 27, 2020
gcf-merge-on-green bot pushed a commit that referenced this pull request Mar 27, 2020
🤖 I have created a release \*beep\* \*boop\* 
---
## [4.0.0](https://github.com/googleapis/release-please/compare/v3.3.0...v4.0.0) (2020-03-27)


### ⚠ BREAKING CHANGES

* upgrade octokit/gts/typescript (#381)

### Code Refactoring

* upgrade octokit/gts/typescript ([#381](https://github.com/googleapis/release-please/issues/381)) ([4e0bf11](https://github.com/googleapis/release-please/commit/4e0bf11b7f4217b5c6b8cd1aa9ea4db1bce06b89))
---


This PR was generated with [Release Please](https://github.com/googleapis/release-please).
yoshi-automation added a commit that referenced this pull request Apr 8, 2020
4e0bf11
commit 4e0bf11
Author: Benjamin E. Coe <[email protected]>
Date:   Fri Mar 27 12:19:26 2020 -0700

    refactor!: upgrade octokit/gts/typescript (#381)
yoshi-automation added a commit that referenced this pull request Apr 10, 2020
Source-Repo: googleapis/release-please
Source-Sha: 4e0bf11
Source-Link: 4e0bf11
Author: Benjamin E. Coe <[email protected]>
Date: Fri Mar 27 12:19:26 2020 -0700
Original-Commit-Message: refactor!: upgrade octokit/gts/typescript (#381)
yoshi-automation added a commit that referenced this pull request Apr 11, 2020
Source-Repo: googleapis/release-please
Source-Sha: 4e0bf11
Source-Link: 4e0bf11
Author: Benjamin E. Coe <[email protected]>
Date: Fri Mar 27 12:19:26 2020 -0700
Original-Commit-Message: refactor!: upgrade octokit/gts/typescript (#381)
yoshi-automation added a commit that referenced this pull request Apr 12, 2020
Source-Repo: googleapis/release-please
Source-Sha: 4e0bf11
Source-Link: 4e0bf11
Author: Benjamin E. Coe <[email protected]>
Date: Fri Mar 27 12:19:26 2020 -0700
Original-Commit-Message: refactor!: upgrade octokit/gts/typescript (#381)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants