-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Added dependencies for issues (#2196) #2531
Changes from 250 commits
c5992f5
021a7e0
55f6cf9
30fe97a
751bbee
913584a
53ec08f
14aa1d2
0bbc695
a651cc3
f188031
58e7c2b
5a9f273
ba734d6
5e4b6fc
5302eca
2fe66c2
a2b652e
762266f
6b3a51a
2dca487
d1df4ec
dd24b9f
827291e
c0bc5e8
993a3c0
3beecd2
4e9e1f8
e364cfc
c4ba8d6
9e9d8b9
80437f8
a54f930
d65fe40
f38b3b4
f1f0607
6109fcf
85cdd94
a57bcf1
fc35252
33c5033
c81041f
db4f578
03769da
6eea6c6
d00eb86
78dd6c1
1fded52
3566e9e
9af3aae
f61ba0f
d0baadc
9dd0cb1
fae4466
8bcb624
25427ae
66433a9
01f9e63
13f0e17
cdd322f
e98f92d
822c75c
f02fd33
7e93434
172b1f0
8354f2b
69769ec
1608248
46f099c
e6aeeac
08763cf
32c5605
ed4a47d
200f88d
9714024
2c8103f
1f0bac0
d29ca0c
70589e6
1064acc
c176353
866beeb
5e20447
2058f4e
5dc9c22
f48d6cd
8931ced
df638db
4855f83
108edf9
2a249c4
c534699
57a4f86
fdb106d
a2a6fed
436ea16
9eedb3f
b7ed097
661bd7c
632cca5
12c0093
96d6db4
0016a3e
cd47077
8735bc2
37dc550
f4cb1c7
8bc0947
86be828
eb295f4
171715b
4d15f28
1f8ef92
0fcc3b2
d5b1c33
5f187df
c44e13b
a5b696a
cf1b573
f71f2d4
4277014
31eff2f
30ae70a
dd5cfbf
fdabcf1
992e168
60834bc
18fe48e
647296c
cd734a3
f837429
0ce2170
9ee04eb
cdb5d65
9e29f71
16b1636
be51a47
0ea51f8
e322fa3
9584d3f
1b63df7
7566292
421c9fa
bd3cf60
b58363c
bcaea09
e842472
64e61d6
637895b
77cf3db
32db1b3
ec0e693
5e0a72c
7d1df8b
b4f406c
d2ab9e2
9313332
e4a6426
138a80b
264f895
0c6f56d
250138f
91e0b53
5e5139f
5852b7c
e1f2a8e
20ed1e9
4d28a82
4f94a01
2950021
c3a5878
7a57b6d
497628e
2edfbcd
a25e1dd
f49dd29
3a24b65
b589726
c3818a9
7b26e2c
3820fc4
2f136fc
29dbc17
6f999a0
7528664
be76631
8d2015f
ee77151
82129b8
b00e209
414d1d0
9e11e77
d0b7307
17b2d43
6af8ef7
2e096a5
1a86536
81b51d0
b258bc0
6a5e138
c0cd0c3
022e9d2
3cd497f
16533e9
0f8a212
a3a544e
8266510
edf661d
9185958
f0406c1
395c062
c877186
288cd40
d29f582
83b8831
617d8cd
6733682
236ad85
fbd78f4
ad353f0
5d8383e
8c1f627
15aa9b1
0da2e34
e18627c
39392de
87f41c0
c52e970
c212a82
990985a
05aaee2
ec9797d
0879a36
425e765
6514da4
12602c2
c760e6a
393013a
2d95ab1
10308a0
d89a136
8a6ba47
f01fdf6
e00e5dd
cf6d426
49c471f
a283309
b6e428d
c3a6210
b8aa1d8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -471,6 +471,10 @@ func UpdateIssuesCommit(doer *User, repo *Repository, commits []*PushCommit) err | |
} | ||
|
||
if err = issue.ChangeStatus(doer, repo, true); err != nil { | ||
// Don't return an error when dependencies are open as this would let the push fail | ||
if IsErrDependenciesLeft(err) { | ||
return nil | ||
} | ||
return err | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But it shows an error message if it cannot be closed because there are open dependencies 🤔 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kolaente Yes, I thinks that's expected? Shouldn't that? |
||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1206,3 +1206,57 @@ func IsErrExternalLoginUserNotExist(err error) bool { | |
func (err ErrExternalLoginUserNotExist) Error() string { | ||
return fmt.Sprintf("external login user link does not exists [userID: %d, loginSourceID: %d]", err.UserID, err.LoginSourceID) | ||
} | ||
|
||
// .___ ________ .___ .__ | ||
// | | ______ ________ __ ____ \______ \ ____ ______ ____ ____ __| _/____ ____ ____ |__| ____ ______ | ||
// | |/ ___// ___/ | \_/ __ \ | | \_/ __ \\____ \_/ __ \ / \ / __ |/ __ \ / \_/ ___\| |/ __ \ / ___/ | ||
// | |\___ \ \___ \| | /\ ___/ | ` \ ___/| |_> > ___/| | \/ /_/ \ ___/| | \ \___| \ ___/ \___ \ | ||
// |___/____ >____ >____/ \___ >_______ /\___ > __/ \___ >___| /\____ |\___ >___| /\___ >__|\___ >____ > | ||
// \/ \/ \/ \/ \/|__| \/ \/ \/ \/ \/ \/ \/ \/ | ||
|
||
// ErrDependencyExists represents a "DependencyAlreadyExists" kind of error. | ||
type ErrDependencyExists struct { | ||
IssueID int64 | ||
DependencyID int64 | ||
} | ||
|
||
// IsErrDependencyExists checks if an error is a ErrDependencyExists. | ||
func IsErrDependencyExists(err error) bool { | ||
_, ok := err.(ErrDependencyExists) | ||
return ok | ||
} | ||
|
||
func (err ErrDependencyExists) Error() string { | ||
return fmt.Sprintf("issue dependency does already exist [issue id: %d, dependency id: %d]", err.IssueID, err.DependencyID) | ||
} | ||
|
||
// ErrCircularDependency represents a "DependencyCircular" kind of error. | ||
type ErrCircularDependency struct { | ||
IssueID int64 | ||
DependencyID int64 | ||
} | ||
|
||
// IsErrCircularDependency checks if an error is a ErrCircularDependency. | ||
func IsErrCircularDependency(err error) bool { | ||
_, ok := err.(ErrCircularDependency) | ||
return ok | ||
} | ||
|
||
func (err ErrCircularDependency) Error() string { | ||
return fmt.Sprintf("cannot create circular dependencies (two issues blocking each other) [issue id: %d, dependency id: %d]", err.IssueID, err.DependencyID) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of "cannot create circular dependencies" it should be like your other errors, something like "circular dependencies exists" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed (as well as the message for dependencies left). |
||
} | ||
|
||
// ErrDependenciesLeft represents an error where the issue you're trying to close still has dependencies left. | ||
type ErrDependenciesLeft struct { | ||
IssueID int64 | ||
} | ||
|
||
// IsErrDependenciesLeft checks if an error is a ErrCircularDependency. | ||
func IsErrDependenciesLeft(err error) bool { | ||
_, ok := err.(ErrDependenciesLeft) | ||
return ok | ||
} | ||
|
||
func (err ErrDependenciesLeft) Error() string { | ||
return fmt.Sprintf("cannot close this issue as it still has open dependencies [issue id: %d]", err.IssueID) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "issue has open dependencies" + fix |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -611,6 +611,19 @@ func (issue *Issue) changeStatus(e *xorm.Session, doer *User, repo *Repository, | |
if issue.IsClosed == isClosed { | ||
return nil | ||
} | ||
|
||
// Check for open dependencies | ||
if isClosed { // only check if we're about to close an issue, otherwise reopening an issue would fail when there are unsatisfied dependencies | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should not check whether there are open deps if |
||
noDeps, err := IssueNoDependenciesLeft(issue) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
if !noDeps { | ||
return ErrDependenciesLeft{issue.ID} | ||
} | ||
} | ||
|
||
issue.IsClosed = isClosed | ||
|
||
if err = updateIssueCols(e, issue, "is_closed"); err != nil { | ||
|
@@ -1492,3 +1505,45 @@ func updateIssue(e Engine, issue *Issue) error { | |
func UpdateIssue(issue *Issue) error { | ||
return updateIssue(x, issue) | ||
} | ||
|
||
// Get Blocked By Dependencies, aka all issues this issue is blocked by. | ||
func (issue *Issue) getBlockedByDependencies(e Engine) (_ []*Issue, err error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO declaring variable names in function "header" (e.g. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed that. |
||
var issueDeps []*Issue | ||
|
||
if err = e. | ||
Table("issue_dependency"). | ||
Select("issue.*"). | ||
Join("INNER", "issue", "issue.id = issue_dependency.dependency_id"). | ||
Where("issue_id = ?", issue.ID). | ||
Find(&issueDeps); err != nil { | ||
return issueDeps, err | ||
} | ||
|
||
return issueDeps, nil | ||
} | ||
|
||
// Get Blocking Dependencies, aka all issues this issue blocks. | ||
func (issue *Issue) getBlockingDependencies(e Engine) ([]*Issue, error) { | ||
var issueDeps []*Issue | ||
|
||
if err := e. | ||
Table("issue_dependency"). | ||
Select("issue.*"). | ||
Join("INNER", "issue", "issue.id = issue_dependency.issue_id"). | ||
Where("dependency_id = ?", issue.ID). | ||
Find(&issueDeps); err != nil { | ||
return issueDeps, err | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems logic, I removed the if and replaced it with a single |
||
} | ||
|
||
return issueDeps, nil | ||
} | ||
|
||
// BlockedByDependencies finds all Dependencies an issue is blocked by | ||
func (issue *Issue) BlockedByDependencies() (_ []*Issue, err error) { | ||
return issue.getBlockedByDependencies(x) | ||
} | ||
|
||
// BlockingDependencies returns all blocking dependencies, aka all other issues a given issue blocks | ||
func (issue *Issue) BlockingDependencies() (_ []*Issue, err error) { | ||
return issue.getBlockingDependencies(x) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -60,6 +60,10 @@ const ( | |
CommentTypeAddTimeManual | ||
// Cancel a stopwatch for time tracking | ||
CommentTypeCancelTracking | ||
// Dependency added | ||
CommentTypeAddDependency | ||
//Dependency removed | ||
CommentTypeRemoveDependency | ||
) | ||
|
||
// CommentTag defines comment tag type | ||
|
@@ -75,23 +79,25 @@ const ( | |
|
||
// Comment represents a comment in commit and issue page. | ||
type Comment struct { | ||
ID int64 `xorm:"pk autoincr"` | ||
Type CommentType | ||
PosterID int64 `xorm:"INDEX"` | ||
Poster *User `xorm:"-"` | ||
IssueID int64 `xorm:"INDEX"` | ||
LabelID int64 | ||
Label *Label `xorm:"-"` | ||
OldMilestoneID int64 | ||
MilestoneID int64 | ||
OldMilestone *Milestone `xorm:"-"` | ||
Milestone *Milestone `xorm:"-"` | ||
OldAssigneeID int64 | ||
AssigneeID int64 | ||
Assignee *User `xorm:"-"` | ||
OldAssignee *User `xorm:"-"` | ||
OldTitle string | ||
NewTitle string | ||
ID int64 `xorm:"pk autoincr"` | ||
Type CommentType | ||
PosterID int64 `xorm:"INDEX"` | ||
Poster *User `xorm:"-"` | ||
IssueID int64 `xorm:"INDEX"` | ||
LabelID int64 | ||
Label *Label `xorm:"-"` | ||
OldMilestoneID int64 | ||
MilestoneID int64 | ||
OldMilestone *Milestone `xorm:"-"` | ||
Milestone *Milestone `xorm:"-"` | ||
OldAssigneeID int64 | ||
AssigneeID int64 | ||
Assignee *User `xorm:"-"` | ||
OldAssignee *User `xorm:"-"` | ||
OldTitle string | ||
NewTitle string | ||
DependentIssueID int64 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You don't need this too. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes i do? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is 2 places with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup. Thats why i need it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I saw that i was wrong with removing I use this to get the Dependent issue details displayed in the comments. But to know which issue i need to get informations about, I also need the ID of the Dependent Issue. And that's where There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've probably started the featue and then forget about it, which is why there was this function (and the corresponding There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This means you can only have 1 dependency right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One per comment. (You can ofc have more than dependency per issue) The ID here is used to get issue details (title, index) later when displaying the comment to be able to link to it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why does Comments track dependencies? 😕 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They don't, the ID is only used to display informations in a comment (title, index, link) when someone adds or removes a dependency. |
||
DependentIssue *Issue `xorm:"-"` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do you need this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To load issue Dependency Details in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But usage of it? I don't see reason to have this field. You never use it. You only set it and save to db, but never read. Or did I overlooked something? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is used in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, loaded, but for what? Where you use it? You load it to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Finally :D There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. well, see my latest comment... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kolaente yes, that added html is the reason I was looking for. I looked at it yesterday and have some suggestions for it too. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what suggestions? |
||
|
||
CommitID int64 | ||
Line int64 | ||
|
@@ -266,6 +272,18 @@ func (c *Comment) LoadAssignees() error { | |
return nil | ||
} | ||
|
||
// LoadDepIssueDetails loads Dependent Issue Details | ||
func (c *Comment) LoadDepIssueDetails() error { | ||
var err error | ||
if c.DependentIssueID > 0 && c.DependentIssue == nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe this is better:
|
||
c.DependentIssue, err = getIssueByID(x, c.DependentIssueID) | ||
if err != nil { | ||
return err | ||
} | ||
} | ||
return nil | ||
} | ||
|
||
// MailParticipants sends new comment emails to repository watchers | ||
// and mentioned people. | ||
func (c *Comment) MailParticipants(e Engine, opType ActionType, issue *Issue) (err error) { | ||
|
@@ -317,22 +335,30 @@ func createComment(e *xorm.Session, opts *CreateCommentOptions) (_ *Comment, err | |
if opts.Label != nil { | ||
LabelID = opts.Label.ID | ||
} | ||
|
||
var depID int64 | ||
if opts.DependentIssue != nil { | ||
depID = opts.DependentIssue.ID | ||
} | ||
|
||
comment := &Comment{ | ||
Type: opts.Type, | ||
PosterID: opts.Doer.ID, | ||
Poster: opts.Doer, | ||
IssueID: opts.Issue.ID, | ||
LabelID: LabelID, | ||
OldMilestoneID: opts.OldMilestoneID, | ||
MilestoneID: opts.MilestoneID, | ||
OldAssigneeID: opts.OldAssigneeID, | ||
AssigneeID: opts.AssigneeID, | ||
CommitID: opts.CommitID, | ||
CommitSHA: opts.CommitSHA, | ||
Line: opts.LineNum, | ||
Content: opts.Content, | ||
OldTitle: opts.OldTitle, | ||
NewTitle: opts.NewTitle, | ||
Type: opts.Type, | ||
PosterID: opts.Doer.ID, | ||
Poster: opts.Doer, | ||
IssueID: opts.Issue.ID, | ||
LabelID: LabelID, | ||
OldMilestoneID: opts.OldMilestoneID, | ||
MilestoneID: opts.MilestoneID, | ||
OldAssigneeID: opts.OldAssigneeID, | ||
AssigneeID: opts.AssigneeID, | ||
CommitID: opts.CommitID, | ||
CommitSHA: opts.CommitSHA, | ||
Line: opts.LineNum, | ||
Content: opts.Content, | ||
OldTitle: opts.OldTitle, | ||
NewTitle: opts.NewTitle, | ||
DependentIssue: opts.DependentIssue, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you need to set There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, you're right, Removed it. |
||
DependentIssueID: depID, | ||
} | ||
if _, err = e.Insert(comment); err != nil { | ||
return nil, err | ||
|
@@ -506,13 +532,30 @@ func createDeleteBranchComment(e *xorm.Session, doer *User, repo *Repository, is | |
}) | ||
} | ||
|
||
// Creates issue dependency comment | ||
func createIssueDependencyComment(e *xorm.Session, doer *User, issue *Issue, dependentIssue *Issue, add bool) (*Comment, error) { | ||
cType := CommentTypeAddDependency | ||
if !add { | ||
cType = CommentTypeRemoveDependency | ||
} | ||
|
||
return createComment(e, &CreateCommentOptions{ | ||
Type: cType, | ||
Doer: doer, | ||
Repo: issue.Repo, | ||
Issue: issue, | ||
DependentIssue: dependentIssue, | ||
}) | ||
} | ||
|
||
// CreateCommentOptions defines options for creating comment | ||
type CreateCommentOptions struct { | ||
Type CommentType | ||
Doer *User | ||
Repo *Repository | ||
Issue *Issue | ||
Label *Label | ||
Type CommentType | ||
Doer *User | ||
Repo *Repository | ||
Issue *Issue | ||
Label *Label | ||
DependentIssue *Issue | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is not dependent issue ID enough? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think it doesn't really make a difference, as it is a pointer... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO you should always use minimum and be exact of what you need. You know, that only DependentIssue.ID will be used here, but not everyone will read the code to learn what it rly needs (and should not). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, I've changed that. When I first started this pr, I used only the IDs everywhere, but I was told to use structs instead, which is why I used that here. |
||
|
||
OldMilestoneID int64 | ||
MilestoneID int64 | ||
|
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.
this should be off by default
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.
I would prefer this be on by default
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.
Mee too. I also think we'd need to be consistent, if we disable this by default we'd need to disable timetracking by default too.
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.
Too many toggles isn't good either. Issue tracking on/off per project I do understand. But if you wish to have issues, why would you want to turn of dependencies? Just don't use it then, like labels. No need to use them. Dependencies on and no ability to turn off. Unless it's not stable, but in that case it should be fixed instead of being toggleable
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.
Made a quick benchmark of load times:
It don't seem to change much if dependencies are enabled or disabled.
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.
@ptman well you can still enable or disable them in repo settings. The only reason I could think of why you want to disable them is because of performance, but as the benchmark shows it doesn't have much of an impact if they are disabled or enabled...
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.
Every feature Gitea implement should not be turned off by default. That should be the highlight of the release. If some one don't want to use it, don't use it then. But to me every big/nice feature like this and time tracking and many more should be on by default. If benchmark is the scale, then i don't see any feature to be added to any application.
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.
@mmarif4u I totally agree with you. And its not very motivating if you create a feature and in the end when its very close to be merged and some people say "sorry but i think we should hide the feature by default so if anyone wants to use it he would need to first know it exists and then activly edit the configuration to enable and use it".
I think it's enough there is the possibility to disable it if one doesn't want to use it. And apparently (see the benchmark) it doesn't really have an impact on load times if it is enabled or disabled.
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.
I agree with @mmarif4u here, as long as there's no performance issue we shouldn't be disabling features. And to extend on it, I don't think that defaults should be in
app.ini
at all but rather stored in the database per User.