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

♻️ Split Onigumo module to smaller parts #90

Open
3 tasks
Glutexo opened this issue May 6, 2022 · 7 comments
Open
3 tasks

♻️ Split Onigumo module to smaller parts #90

Glutexo opened this issue May 6, 2022 · 7 comments

Comments

@Glutexo
Copy link
Owner

Glutexo commented May 6, 2022

  • URLs file reader
  • HTTP client, response extractor
  • Integrating downloader
@Glutexo
Copy link
Owner Author

Glutexo commented Jul 25, 2022

What parts? Ideas, @nappex?

I see that we can extract the parts related to HTTP. That is, get_url and get_body. But is it necessary? Maybe not now, when there are still just a handful of short and straightforward methods.

If you don’t object, I will close this issue for now.

@Glutexo
Copy link
Owner Author

Glutexo commented Jul 25, 2022

Renaming the Onigumo module to Downloader may help. The original name hints it is a general-purpose module. The methods inside are, however, specific, related to downloading. See #118.

@Glutexo
Copy link
Owner Author

Glutexo commented Jul 25, 2022

I am closing this issue. Feel free to re-open if you have a clear idea of what to extract from the module or want to spike about it.

@Glutexo Glutexo closed this as completed Jul 25, 2022
@Glutexo Glutexo closed this as not planned Won't fix, can't repro, duplicate, stale Jul 25, 2022
@nappex
Copy link
Collaborator

nappex commented Jul 25, 2022

i suppose that main idea was to seperate code to more func. if you remember we did a lot work inside one func but i feel we ve already fixed the problem. yes i agree it is a general problem, kind of refactor which can be done any time...

@Glutexo
Copy link
Owner Author

Glutexo commented Aug 14, 2022

I’ve put more thought into this and see three possible descendants of the Downloader module:

  • URLs file reader. The input file may become more complex than a simple list of addresses.
  • HTTP client. A wrapper around the HTTPoison library issuing a request and extracting the response body. *
  • The Downloader. This integration part would put all the functionality together.

* I see the “and” there, which may point to the module doing two things instead of one. I think, however, that the wrapping around HTTPoison is so thin that it does not deserve further extraction.

@Glutexo Glutexo reopened this Aug 14, 2022
@Glutexo
Copy link
Owner Author

Glutexo commented Aug 14, 2022

I reopened the issue and added the proposed parts to the description. I am looking forward to your feedback, @nappex.

@Glutexo
Copy link
Owner Author

Glutexo commented Aug 14, 2022

I realized that there is the file name creating function. It looks separate, but now I think it belongs to the integration Downloader module.

@Glutexo Glutexo changed the title Split Onigumo module to smaller parts ♻️ Split Onigumo module to smaller parts Aug 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants