refactor and add tests

[rgalonso]

This contains some additional minor refactoring and adds another unittest file.

refactor-related changes

The following changes were made further isolate the VCS-specific code from that in main.

• added abstract Client base class
  • this also supports mypy-related changes, discussed below
• parse MANUAL_COMMIT_REF and MANUAL_BASE_REF environment variables within *Client so that diff URL can be VCS-specific
  • for LocalClient, there is no URL, but parsing these variables allows it to call git diff with the proper arguments
• Client:get_issue_url() returns just the URL string (when applicable), rather than prefixing it with "Issue URL: ". This allows the function to be used more generically.

more thorough testing

In addition, per this discussion in PR #217, I added an additional test file which uses mypy to look for additional errors that weren't being caught before. mypy is really intended for layering static typing on top of Python. That's a nice goal, but I've limited the changes here to most just focus on having the additional testing without being concerned about the typing.

other minor items

• added devcontainer configuration
  • to support local development
• added additional (non-defualt) Dockerfile target
  • to support local testing and upcoming GitLab integration
• minor changes to requirements/-dev.txt and GitHub workflow to support above changes

[alstr]

This is awesome. Thanks very much for the effort. I'll take a look over it shortly!

[rgalonso]

This is awesome. Thanks very much for the effort. I'll take a look over it shortly!

In particular, please pay attention to how this is working for GitHub Actions. Since I'm focused on GitLab, I'm not testing the GitHub side of it all. I'm not even sure how to! So I'm definitely relying on you and any existing regression tests to make sure that side of it is still working properly. (You can probably take that as a general note as I continue to submit new PRs.) Thanks again!

[alstr]

All looks good, thanks again. About the update to the Dockerfile; what's the purpose of the additional stage? Just wondering if it'll slow things down.

[rgalonso]

That's a fair concern, but with modern Docker it's not an issue. It used to be that it would build all stages, but these days it's smart enough to skip the new stage I introduced because it can see that the final (unnamed) stage doesn't rely on it.

As to why I introduced it, it's needed for GitLab CI/CD. Independent of that, I've also found it useful for local testing prior to a push.

Get Outlook for Androidhttps://aka.ms/AAb9ysg

---

From: Alastair Mooney @.> Sent: Monday, November 4, 2024 7:05:36 AM To: alstr/todo-to-issue-action @.> Cc: Robert Alonso @.>; Author @.> Subject: Re: [alstr/todo-to-issue-action] refactor and add tests (PR #222)

Caution: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.

All looks good, thanks again. About the update to the Dockerfile; what's the purpose of the additional stage? Just wondering if it'll slow things down.

— Reply to this email directly, view it on GitHubhttps://github.com/alstr/todo-to-issue-action/pull/222#issuecomment-2454540832, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AEFHTTIK4HW7MNGRGADUH5LZ65PIZAVCNFSM6AAAAABQYDFAXGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINJUGU2DAOBTGI. You are receiving this because you authored the thread.Message ID: @.***>