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.

Sign up for GitHub

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

Jump to bottom

style+lint: webi_download should always use 2-3 args #721

Merged
merged 2 commits into from
Oct 31, 2023
Merged

style+lint: webi_download should always use 2-3 args #721

merged 2 commits into from
Oct 31, 2023

Conversation

coolaj86
Copy link
Member

@coolaj86 coolaj86 commented Oct 30, 2023 •
edited
Loading

Simplifying the webi_download function and using it consintently.

  • separates error check
  • easier to grok
  • easier to grep -A 2

@coolaj86 coolaj86 changed the title lint args: webi_download should always use 2-3 args style+lint: webi_download should always use 2-3 args Oct 30, 2023
@coolaj86 coolaj86 changed the base branch from main to ref-internals October 30, 2023 08:03
@coolaj86 coolaj86 requested a review from HacDan October 30, 2023 17:34
HacDan
HacDan approved these changes Oct 30, 2023
View reviewed changes
Copy link
Collaborator

@HacDan HacDan left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for refactoring this!

@coolaj86
Copy link
Member Author

coolaj86 commented Oct 30, 2023 •
edited
Loading

It's necessary for adding git as a download method, cleanly:
https://github.com/webinstall/webi-installers/pull/718/files#diff-d3fbda26bbb7d3b1a8068eed2a5c1e0a86d864a8fbebc3b0d01e09ff176c7ed0

I'll pull it in after #720 is reviewed and puled in.
(there's no dependency, I just wanted to split the two and arbitrarily based this on that)

Base automatically changed from ref-internals to main October 31, 2023 00:04
@coolaj86 coolaj86 merged commit 4359bfd into main Oct 31, 2023
@coolaj86 coolaj86 deleted the lint-args branch October 31, 2023 00:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@HacDan HacDan HacDan approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@coolaj86 @HacDan