-
-
Notifications
You must be signed in to change notification settings - Fork 6
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
Semantic Tasks 2.1.0 (KM-A) #53
Conversation
1st update
method_exists( RevisionRecord::class, 'getPage' )
updating credits
fix type
prepare V 3 release
Assignees.php
Outdated
use WikiPage; | ||
|
||
/** @todo: rename TaskDiff something similar */ | ||
class Assignees { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you copy this class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hello Jeroen, in the root folder ? it was an error of github file upload, you can delete it
Anything we can do to get this accepted? |
The file was already deleted by Thomas, so I guess this is ready for acceptance? |
I suggest to add GitHub Actions CI and PHPStan to help with reviews. @Jereon: shall we merge anyhow already and ask @thomas-topway-it and @krabina to improve testing in a separate PR? |
This iis my preferred approach. Since we don't have test coverage up to now, I don't want to have too comples PRs. ST is not working in the current versions without our PR and we can always improve test coverage later. Also, we have to learn how to provide high quality PRs to make life as easy as possible for those with write access. |
Hello. I want to ask a couple of questions. Why am I tagged in this
email? Am I related to your dialogue?
чт, 6 окт. 2022 г., 11:59 Bernhard Krabina ***@***.***>:
… This iis my preferred approach. Since we don't have test coverage up to
now, I don't want to have too comples PRs. ST is not working in the current
versions without our PR and we can always improve test coverage later.
Also, we have to learn how to provide high quality PRs to make life as easy
as possible for those with write access.
—
Reply to this email directly, view it on GitHub
<#53 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AVW7K4C7B5D563VFWT7XMLTWB2IF5ANCNFSM6AAAAAAQLE4W4A>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@Jereon they meant to ping me @krabina well how do we know this code does what it is intended to do? how do we know it does not break key existing functionality? You can't reliably verify that just by looking at the code. We could YOLO merge... Especially if you manually tested. Perhaps that is the best we can do for this project. |
I guess since #52 is blocking usage in SMW, it is best for this repo to accept this. Or wait for others to test our fork? I have it in production in two wikis, one is still on SMW 3.x, the other in SMW 4. We are willing to add testing in the future. Maybe even in the next weeks. Would be nice if someone who also uses this would test our version. |
This PR is made in reference to: KM-A Knowledge Management Associates upgrade of Semantic Tasks
This PR addresses or contains:
See https://github.com/Knowledge-Wiki/SemanticTasks/blob/master/docs/RELEASE-NOTES.md
This PR includes: