Skip to content
This repository has been archived by the owner on Mar 25, 2023. It is now read-only.

Create Exception module for error value #25

Open
SimonLab opened this issue May 24, 2022 · 5 comments
Open

Create Exception module for error value #25

SimonLab opened this issue May 24, 2022 · 5 comments
Assignees
Labels
awaiting-review An issue or pull request that needs to be reviewed enhancement New feature or request T1h Time Estimate 1 Hour

Comments

@SimonLab
Copy link
Member

SimonLab commented May 24, 2022

Go over the functions' return values and make sure they are consistent by returning either {:ok, value} or {:error, exception}
This will make it easier for applications using the library to pattern match on these functions.

For the exception we can create our own exception module, see https://elixir-lang.org/getting-started/try-catch-and-rescue.html#errors

I like the way the comment on this article describe how to use "custom" exception as return value for errors, and how it work nicely with the with statement using the :reason key from the exception module, https://michal.muskala.eu/post/error-handling-in-elixir-libraries/

see also https://medium.com/@moxicon/elixir-best-practices-for-error-values-50dc015a06f5

@SimonLab SimonLab added enhancement New feature or request T1h Time Estimate 1 Hour labels May 24, 2022
@nelsonic nelsonic added the help wanted Extra attention is needed label May 24, 2022
@SimonLab SimonLab self-assigned this May 24, 2022
@SimonLab SimonLab added the in-progress An issue or pull request that is being worked on by the assigned person label May 24, 2022
SimonLab added a commit that referenced this issue May 24, 2022
Create custom exception to return in the function calls

related to: #24 #25
@SimonLab SimonLab added awaiting-review An issue or pull request that needs to be reviewed and removed help wanted Extra attention is needed in-progress An issue or pull request that is being worked on by the assigned person labels May 24, 2022
@nelsonic
Copy link
Member

@SimonLab I merged your PR #26
I get the following failed tests on main when attempting to run the tests with mock: true:

  1) test commit/2 creates a commit in the repo (GiteaTest)
     test/gitea_test.exs:169
     ** (MatchError) no match of right hand side value: {:error, %Git.Error{args: ["."], code: 1, command: "add", message: "The following paths are ignored by one of your .gitignore files:\ntmp\nhint: Use -f if you really want to add them.\nhint: Turn this message off by running\nhint: \"git config advice.addIgnoredFile false\"\n"}}
     code: Gitea.commit(org_name, repo_name, %{
     stacktrace:
       (gitea 1.0.5) lib/gitea.ex:227: Gitea.commit/3
       test/gitea_test.exs:182: (test)

  2) test Gitea.push/2 pushes the commit to the remote repo (GiteaTest)
     test/gitea_test.exs:195
     ** (MatchError) no match of right hand side value: {:error, %Git.Error{args: ["."], code: 1, command: "add", message: "The following paths are ignored by one of your .gitignore files:\ntmp\nhint: Use -f if you really want to add them.\nhint: Turn this message off by running\nhint: \"git config advice.addIgnoredFile false\"\n"}}
     code: Gitea.commit(org_name, repo_name, %{
     stacktrace:
       (gitea 1.0.5) lib/gitea.ex:227: Gitea.commit/3
       test/gitea_test.exs:209: (test)

I did a fresh git clone of the repo ... 🆕
Could you please confirm that the tests pass on your localhost? 🙏
They appear to be fine on GitHub CI ... 💭

@nelsonic
Copy link
Member

Meanwhile package published to https://hex.pm/packages/gitea/1.0.5 📦 🚀 (to not delay your work)

@SimonLab
Copy link
Member Author

Tested again on a fresh clone, I have the same errors.
However I don't think it's link to might latest PR as I've tested on the v1.0.3 and have similar errors.
I think it might be linked to the .gitignore having tmp folder ignore:

gitea/.gitignore

Lines 25 to 28 in 5dfdb2c

# Temporary files, for example, from tests.
/tmp/
tmp/test-repo
tmp/test-repo/README.md

but

# Optionally set the path where you want to clone git repos:
export GIT_TEMP_DIR_PATH=tmp

is using tmp

@nelsonic
Copy link
Member

@SimonLab any ideas what we can do to overcome this?
We didn't have this issue on gogs because of the submodule.

@nelsonic
Copy link
Member

Dedicated issue for creating submodule: #29

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
awaiting-review An issue or pull request that needs to be reviewed enhancement New feature or request T1h Time Estimate 1 Hour
Projects
None yet
Development

No branches or pull requests

2 participants