-
-
Notifications
You must be signed in to change notification settings - Fork 617
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
chore: refactor code to eliminate redundancy #972
Conversation
7ccc417
to
6170fd0
Compare
@evenstensberg |
This change is good |
@jamesgeorge007 your change broke some test. This means that your new logic has some flaw or the tests have to be revisited. Check the log of the CI and make sure all the tests are passing on your machine. |
6170fd0
to
07edfbd
Compare
07edfbd
to
95ad542
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but tests are failing, need to look into that
@evenstensberg The following test script for detecting yarn as the package manager fails each and every time.
|
Could you fix those or as a last resort disable them? |
@jamesgeorge007 Thanks for your update. I labeled the Pull Request so reviewers will review it again. @evenstensberg Please review the new changes. |
@jamesgeorge007 Please review the following output log for errors:
See complete report here. |
@evenstensberg recently I rebased my branch from |
Aaah yes. We should disable those tests for now. Could you be kind to open a PR that disables them before we merge this one? |
@evenstensberg So what you intend is to open yet another PR with 72cfdfa 🤔 |
You can do |
@evenstensberg I guess you're expecting any new PR's once all the tests are passing on the |
I changed a branch to a new next clone. Could you reopen? I'm currently updating tests there now |
Gotta fix the src before we can review the PR's, reopening but will not review yet. |
72cfdfa
to
73abfbb
Compare
05a0339
to
f0031fd
Compare
Check out #1002 |
What kind of change does this PR introduce?
refactoring
Did you add tests for your changes?
Nope
If relevant, did you update the documentation?
N/A
Summary
Made a minor refactor so as to eliminate redundancy.
N/A
Does this PR introduce a breaking change?
Nope
Other information
N/A