Skip to content
This repository has been archived by the owner on Sep 9, 2020. It is now read-only.

[WIP] Add runOld method to statusCommand #1402

Closed

Conversation

al3rez
Copy link

@al3rez al3rez commented Nov 25, 2017

What does this do / why do we need it?

To list the dependencies that are out-of-date by comparing
revesion of each lock projects to the solution one.

What should your reviewer look out for in this PR?

Comparing revision of each project logic

Do you need help or clarification on anything?

How can I write a test for this case?

Which issue(s) does this PR fix?

fixes #1383

Why?
To list the dependencies that are out-of-date by comparing
revesion of each lock projects to the solution one.
@googlebot
Copy link
Collaborator

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot.
  • In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again.

@al3rez
Copy link
Author

al3rez commented Nov 25, 2017

@googlebot I signed it!

@googlebot
Copy link
Collaborator

CLAs look good, thanks!

Copy link
Collaborator

@darkowlzz darkowlzz left a comment

Choose a reason for hiding this comment

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

Looks to be in the right direction.
Let's call it from cmd.Run and see the result.
Also, we need an integration test for this, similar to those in cmd/dep/testdata/harness_tests/status.

@@ -273,6 +274,36 @@ func (cmd *statusCommand) Run(ctx *dep.Ctx, args []string) error {
return nil
}

func (cmd *statusCommand) runOld(ctx *dep.Ctx, args []string, p *dep.Project, sm gps.SourceManager, params gps.SolveParameters) error {
solver, err := gps.Prepare(params, sm)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since we want to check update for all the projects, set params.ChangeAll = true before preparing a solver.

}
}

// TODO: Print output
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can print to ctx.Out for now. Maybe we would like to use some outputter later.

ctx.Out.Println("print some text to stdout")


for _, sp := range solutionProjects {
for _, lp := range lockProjects {
spr, _, _ := gps.VersionComponentStrings(sp.Version())
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can be moved out to the outer loop to reduce the number of times we calculate this.

Alireza Bashiri added 5 commits November 26, 2017 17:31
How?
By looping just through solution projects and use its index to access 
both each lock project and  solution project.
Why?
Because it's not available in Run method and to follow runStatusAll
initialize a gps.SolveParameters of inside the method itself.
Why?
To be able to do `dep status --old`
@darkowlzz
Copy link
Collaborator

@azbshiri ping! need help with the tests or anything?

@al3rez
Copy link
Author

al3rez commented Jan 15, 2018

@darkowlzz yeah I got stuck with functional test and I've been busy. It'd be great if you could help me out.

@darkowlzz
Copy link
Collaborator

Alright, first you should complete the implementation. The existing code looks incomplete. There's OldStatus type but it's not used to print the result. You have to implement tableOutput, jsonOutput and templateOutput of OldStatus.

Once that is ready, integration tests can be easily written against the output from the above implementation, like other tests in cmd/dep/testdata/harness_tests/status/.

Hope that makes sense. If you think you won't have time to work on it, it's fine, just leave a note and we will take care of it 🙂

@al3rez
Copy link
Author

al3rez commented Jan 19, 2018

yeah, I don't think I'd have time to work on this.

@darkowlzz
Copy link
Collaborator

@azbshiri thanks, it's alright 🙂

@zkry you might be interested in this.

@zkry
Copy link
Contributor

zkry commented Jan 21, 2018

@darkowlzz Yeah, I am interested 🙂. Been looking for opportunities to contribute lately. I'll take a look at this.

@zkry zkry mentioned this pull request Jan 23, 2018
@zkry
Copy link
Contributor

zkry commented Jan 23, 2018

I made a new PR here #1553 so I can work from my fork.

@darkowlzz
Copy link
Collaborator

Closing this with #1553 merged.

@darkowlzz darkowlzz closed this Apr 1, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement -old flag for dep status
4 participants