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

[bug] the commit "fix: not use ref values to transform" failed the CI tests #59

Closed
hsluoyz opened this issue Apr 1, 2023 · 6 comments
Closed
Assignees
Labels
bug Something isn't working

Comments

@hsluoyz
Copy link
Member

hsluoyz commented Apr 1, 2023

This commit: cf65b40 has introduced bugs and caused the CI tests to fail: https://github.com/casbin-net/casbin-aspnetcore/actions/runs/4470605463/jobs/7854387801 . This commit has been reverted and backed-up at another branch: https://github.com/casbin-net/casbin-aspnetcore/tree/sagilio

@sagilio next time, please make a PR so we can track whether the code has broken any CI

@casbin-bot
Copy link

@casbin-bot casbin-bot added the bug Something isn't working label Apr 1, 2023
@hsluoyz hsluoyz closed this as completed Apr 1, 2023
@sagilio
Copy link
Member

sagilio commented Apr 1, 2023

Ok. I will fix it later. This commit will fix another BUG, we can not reset it.

@hsluoyz
Copy link
Member Author

hsluoyz commented Apr 2, 2023

@sagilio why did you remove my fix for: #58 (comment) ? The fix commit: d0fa72b is not in any branch now

@sagilio
Copy link
Member

sagilio commented Apr 2, 2023

@sagilio why did you remove my fix for: #58 (comment) ? The fix commit: d0fa72b is not in any branch now

@hsluoyz

p, [email protected], BasicTest, Get

This policy is an example of the basic transformer, it is not wrong here.
it needs the user to change the default transformer options to test it. I commented on the reason here (#58 (comment)).

In fact, I have tried to enhance the experience for the user trying the example by setting the transformer at the attribute. But the change is in the reverted commit and there is a little logic conflict with your commit

So I recovered the backup branch and only cherry-pick the last commit to make sure the last version worked well.

@hsluoyz
Copy link
Member Author

hsluoyz commented Apr 2, 2023

it needs the user to change the default transformer options to test it. I commented on the reason here (#58 (comment)).

I don't think it makes sense if it has made a lot of people think it's a bug. It's not a good example code. If you want to show some special transformer functionality, then you create a "working" example especially for it. The example itself SHOULD be working out-of-box. Above all, the result we see is that the user experience has been impacted by the wrongly-designed example, long-time no response in the issue, and broken CI for several months. I don't think this is a good way to maintain the software responsibly.

@sagilio
Copy link
Member

sagilio commented Apr 2, 2023

it needs the user to change the default transformer options to test it. I commented on the reason here (#58 (comment)).

I don't think it makes sense if it has made a lot of people think it's a bug. It's not a good example code. If you want to show some special transformer functionality, then you create a "working" example especially for it. The example itself SHOULD be working out-of-box. Above all, the result we see is that the user experience has been impacted by the wrongly-designed example, long-time no response in the issue, and broken CI for several months. I don't think this is a good way to maintain the software responsibly.

You are right, the example code needs to enhance and I'm also sorry for the lack of time to positively maintain it lately, I will improve it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants