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

Semantic Tasks 2.1.0 (KM-A) #53

Merged
merged 40 commits into from
Oct 8, 2022

Conversation

thomas-topway-it
Copy link
Contributor

This PR is made in reference to: KM-A Knowledge Management Associates upgrade of Semantic Tasks
This PR addresses or contains:

  • fix issues/52
  • fix issues/49
  • fix issues/39
  • fix issues/38

See https://github.com/Knowledge-Wiki/SemanticTasks/blob/master/docs/RELEASE-NOTES.md

This PR includes:

  • Tests (unit/integration)
  • CI build passed

@thomas-topway-it
Copy link
Contributor Author

@krabina
Copy link
Contributor

krabina commented Sep 13, 2022

This PR fixes

#52
#49
#39
#38

Assignees.php Outdated
use WikiPage;

/** @todo: rename TaskDiff something similar */
class Assignees {
Copy link
Member

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?

Copy link
Contributor Author

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

@krabina
Copy link
Contributor

krabina commented Sep 28, 2022

Anything we can do to get this accepted?

@krabina
Copy link
Contributor

krabina commented Oct 6, 2022

The file was already deleted by Thomas, so I guess this is ready for acceptance?

@gesinn-it-gea
Copy link
Member

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?

@krabina
Copy link
Contributor

krabina commented Oct 6, 2022

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.

@Jereon
Copy link

Jereon commented Oct 8, 2022 via email

@JeroenDeDauw
Copy link
Member

@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.

@krabina
Copy link
Contributor

krabina commented Oct 8, 2022

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.

@JeroenDeDauw JeroenDeDauw merged commit 1b90e99 into SemanticMediaWiki:master Oct 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants