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

package will now be listed, sorted by lastupdatedtime #1334

Merged
merged 4 commits into from Oct 1, 2019
Merged

package will now be listed, sorted by lastupdatedtime #1334

merged 4 commits into from Oct 1, 2019

Conversation

viveksinghggits
Copy link
Contributor

@viveksinghggits viveksinghggits commented Sep 30, 2019

This change fixes #1093


This change is Reviewable

@viveksinghggits viveksinghggits changed the title package will not be listed, sorted by lastupdatedtime package will now be listed, sorted by lastupdatedtime Sep 30, 2019
Copy link
Member

@life1347 life1347 left a comment

Choose a reason for hiding this comment

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

Some minor issues

pkg/apis/fission.io/v1/types.go Show resolved Hide resolved
pkg/fission-cli/package.go Outdated Show resolved Hide resolved
pkg/fission-cli/package.go Show resolved Hide resolved
@viveksinghggits
Copy link
Contributor Author

Hi @life1347
I have commited the new changes.

@life1347
Copy link
Member

life1347 commented Oct 1, 2019

LGTM

@life1347 life1347 merged commit 96ae708 into fission:master Oct 1, 2019
@viveksinghggits
Copy link
Contributor Author

Hi @life1347,
if possible can you please check if this fix is working as expected, I am recently seeing some issues in this.
Last update time is not being set for the packages.

@life1347
Copy link
Member

Hi @viveksinghggits

The packages are sorted, but the problem is that the print last update timestamp change was missing in PR #1341 . One interesting thing I found is that in each commit of PR you couldn't see any deletion of the print last update timestamp, but you can only see the changes at the overall changes. I guess that's why I missed it.

Sorry I didn't review the PR carefully. I will raise a PR to fix the problem.

@viveksinghggits
Copy link
Contributor Author

Hi @life1347,
do you want me to do something about this?

@life1347
Copy link
Member

@viveksinghggits I just opened a PR (#1364) to resolve the problem. Thanks for your feedback and I will be more careful next time when reviewing the PR.

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.

Add (and sort by) timestamps to pkg list
2 participants