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

feat: adding support for using permsRepo #76

Merged
merged 5 commits into from
Jul 16, 2020
Merged

feat: adding support for using permsRepo #76

merged 5 commits into from
Jul 16, 2020

Conversation

soldair
Copy link
Collaborator

@soldair soldair commented Jul 9, 2020

uses permsRepo instead of repository to check for publish permissions

Change-Id: I59ae3e9defc70855b1fe0d385949a93437dcb528

uses permsRepo instead of repository to check for publish permissions

Change-Id: I59ae3e9defc70855b1fe0d385949a93437dcb528
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 9, 2020
@soldair soldair requested a review from bcoe July 9, 2020 21:25
@soldair
Copy link
Collaborator Author

soldair commented Jul 9, 2020

lint is going to be so angry. just like it has been already

@codecov
Copy link

codecov bot commented Jul 9, 2020

Codecov Report

Merging #76 into master will decrease coverage by 0.02%.
The diff coverage is 40.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #76      +/-   ##
==========================================
- Coverage   29.03%   29.01%   -0.03%     
==========================================
  Files          18       18              
  Lines        2228     2230       +2     
  Branches      105       92      -13     
==========================================
  Hits          647      647              
- Misses       1581     1583       +2     
Impacted Files Coverage Δ
src/lib/write-package.ts 72.81% <40.00%> (-0.48%) ⬇️

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 6f2c307...4440796. Read the comment docs.

Change-Id: I7d3bc9d03c3c1f8e41af0a6fcde9400817be1862
test/lib/write-package.ts Outdated Show resolved Hide resolved
@@ -118,13 +118,15 @@ export const writePackage = async (
}

console.info('latest repo ', latest.repository);

const repo = repoToGithub(latest.repository);
//tslint:disable-next-line:no-any
Copy link
Contributor

Choose a reason for hiding this comment

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

The format for eslint here is different, which is why you're still getting the linter warning.

// eslint-disable-next-line @typescript-eslint/no-explicit-any

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ah thanks i gave up after googling and stiff drink

test/lib/write-package.ts Outdated Show resolved Hide resolved
@bcoe
Copy link
Contributor

bcoe commented Jul 15, 2020

👋 the unused variables linting rule is just a warning, let's not go out of our way to call the variables (as it creates a bit of confusion), and let's simply take on the warning for now.

However, I have a fix for the warning I've tested here.

Change-Id: I876cd9507b31dc9724cb0ae66cfcac2f142bfcd4
Change-Id: Ia0cfe8dfbe4617ed81307c215ba5028bd112d152
Copy link
Contributor

@bcoe bcoe left a comment

Choose a reason for hiding this comment

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

LGTM, should I deploy tomorrow when it's a bit earlier in the day?

@bcoe bcoe merged commit 782665e into master Jul 16, 2020
@bcoe bcoe deleted the perms-repo branch July 16, 2020 00:05
@soldair
Copy link
Collaborator Author

soldair commented Jul 16, 2020

just let me know and ill try it out =)

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