Add/checkout optional git path #24
Conversation
If the user is running on GitHub actions, it is most likely that a separate clone is not necessary, but instead we parse content in the git repository base directory. If a git_path and branch are provided, then we instead clone these paths and proceed as we did before. Additionally, since deletion is not always required, we add a cleanup variable that defaults to false (we dont delete the repository uless requested by the user Signed-off-by: vsoch <vsochat@stanford.edu>
Im also adding a subfolder argument so the user can select a subfolder of the repository to check. In practice, if you parse an entire repository for urls you find that there are some strings that should not be checked (eg something like "https://%s" %path Signed-off-by: vsoch <vsochat@stanford.edu>
Testing has shown that some sites return 403 (Forbidden) if its not an okay looking (human browser) user agent. Thus, we add a function to derive a user agent and use it for the requests Signed-off-by: vsoch <vsochat@stanford.edu>
|
@SuperKogito the tests are timing out and I'm not sure why. When you get a chance, could you take a look and give me a sense of what might be the issue? There is no output whatsoever so it's hard to tell. The action functions fine in my real world test case, so I'm wondering if a test needs to be updated for my changes. |
| deletion_status = del_repo(base_path) | ||
| # delete repo when done, if requested | ||
| if cleanup == "true": | ||
| del_repo(base_path) | ||
|
|
There was a problem hiding this comment.
I am not sure if I get this change.
We usually clone the repo, run checks and the ideal final step is to clean up by removing the cloned repo so why would the user would want to keep the clone? can you please explain the logic behind it?
There was a problem hiding this comment.
Most users that use the action will get the repo by way of the github/actions/checkout step run before, in which case the repository isn't cloned, but the base path used is that checked out repo. 95% of users will not want the check to be run and then have their repository deleted. Let me know if this is clear or if you need clarification on any point.
There was a problem hiding this comment.
Here is an explicit, real world example. If I were to delete the repository after running the URL check step, none of the testing steps that follow would work.
steps:
- uses: actions/checkout@v2
- name: URLs-checker
uses: urlstechie/URLs-checker@add/checkout-optional-git-path
with:
subfolder: docs
- name: Run Tests
run: |
# This (and anything following) will not work if URLs-checker deletes my repo!
pytest tests/*.py
|
I like some of the new changes but I don't get some. Overall, I think the PR is a good improvement. I mentioned some changes I would like to have. We can discuss them here of course. |
|
Sounds good! There is definitely no rush, and post any questions that you have here, I'll find them :) |
This pull request will allow for easier usage of the url checker on GitHub actions, meaning that the git_path is optional, and if not provided, we use the checkout of the branch that is already done by default. Specifically:
subfolderargument, the reason being that after clone of a git path (or use of the already checked out repository) we might want to limit checks to a particular folder. In my testing case, I only wanted to parse files in docs/You can see the branch in action here: https://github.com/HPC-buildtest/buildtest-framework/pull/211/checks?check_run_id=501529549