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

Consolidate list of apps and reduce repetition #161

Merged
merged 1 commit into from
Oct 21, 2020

Conversation

Waterdrips
Copy link
Contributor

@Waterdrips Waterdrips commented Jul 27, 2020

Description

To add an app previously it was very easy to miss adding it's info to the
info command. This changes the way of adding an app to building a list of
app "objects" and then using this list to get the info message and the
installer command.

I found a few apps which hadn't been added to the info command, probably because it was missed as its yet another thing to add!

Signed-off-by: Alistair Hey alistair@heyal.co.uk

Motivation and Context

How Has This Been Tested?

running install and info on apps.

worked as before

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I've read the CONTRIBUTION guide
  • I have signed-off my commits with git commit -s
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have tested this on arm, or have added code to prevent deployment

@Waterdrips Waterdrips force-pushed the waterdrips-simplify-app-list branch from 12ae896 to fe7d218 Compare July 27, 2020 15:47
@Waterdrips
Copy link
Contributor Author

@alexellis WDYT?

cmd/install.go Outdated Show resolved Hide resolved
@Waterdrips Waterdrips force-pushed the waterdrips-simplify-app-list branch 3 times, most recently from 97766af to a1547bc Compare August 16, 2020 17:05
@Waterdrips Waterdrips force-pushed the waterdrips-simplify-app-list branch 2 times, most recently from abd7ae4 to 3fc8aad Compare October 12, 2020 07:18
cmd/info.go Outdated Show resolved Hide resolved
cmd/install.go Outdated Show resolved Hide resolved
Copy link
Owner

@alexellis alexellis left a comment

Choose a reason for hiding this comment

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

I think this is moving in the right direction. I've left some comments for your consideration.

@Waterdrips
Copy link
Contributor Author

Thanks, Ill continue to work on it based on the comments

@Waterdrips
Copy link
Contributor Author

Requested changes implemented

@Waterdrips Waterdrips force-pushed the waterdrips-simplify-app-list branch 2 times, most recently from 5843643 to a3b6223 Compare October 16, 2020 06:35
To add an app previously it was very easy to miss adding it's info to the
info command. This changes the way of adding an app to building a list of
app "objects" and then using this list to get the info message and the
installer command.

Signed-off-by: Alistair Hey <alistair@heyal.co.uk>
Copy link
Owner

@alexellis alexellis left a comment

Choose a reason for hiding this comment

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

Approved

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.

None yet

2 participants