Skip to content

[IAppConfig] new tests #43370

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.

Sign up for GitHub

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

Jump to bottom
Merged
ArtificialOwl merged 6 commits into from
Feb 12, 2024
Merged

[IAppConfig] new tests #43370

ArtificialOwl merged 6 commits into from
Feb 12, 2024

Conversation

@ArtificialOwl
Copy link
Member

@ArtificialOwl ArtificialOwl commented Feb 6, 2024 •
edited
Loading

  • moving previous test into another test file
  • new tests for the new API IAppConfig
  • fix the search on app id (prefix)
  • remove useless check on key, redundant with isLazy()

@ArtificialOwl ArtificialOwl force-pushed the test-iappconfig branch 2 times, most recently from 543b714 to 7b09113 Compare February 6, 2024 01:44
@ArtificialOwl ArtificialOwl added this to the Nextcloud 29 milestone Feb 6, 2024
@ArtificialOwl ArtificialOwl added the 3. to review Waiting for reviews label Feb 6, 2024
This was referenced Feb 6, 2024
Merged
Merged
come-nc
come-nc approved these changes Feb 6, 2024
View reviewed changes
Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have to keep a deprecated test class around? Simply remove it, no?

@ArtificialOwl
Copy link
Member Author

ArtificialOwl commented Feb 6, 2024

Do we have to keep a deprecated test class around? Simply remove it, no?

was thinking to keep them for a while to ensure compatibility of the deprecated methods

nickvergessen
nickvergessen reviewed Feb 6, 2024
View reviewed changes
@ArtificialOwl
new API, new tests
44b76cb 
Signed-off-by: Maxence Lange <[email protected]>
emoral435
emoral435 approved these changes Feb 12, 2024
View reviewed changes
Copy link
Contributor

@emoral435 emoral435 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside from small nitpick, and me agreeing with @come-nc that we do not really need to keep the old tests, everything looks good to me. But I do agree that we really should remove it, especially if it is not being used.

@ArtificialOwl
removing deprecated tests
52ebb8e 
Signed-off-by: Maxence Lange <[email protected]>
@ArtificialOwl
fix phpdoc
809e0fe 
Signed-off-by: Maxence Lange <[email protected]>
@ArtificialOwl
Copy link
Member Author

ArtificialOwl commented Feb 12, 2024

  • fixed phpdoc
  • removing deprecated tests

@ArtificialOwl
Copy link
Member Author

ArtificialOwl commented Feb 12, 2024

(failing check not related)

@ArtificialOwl ArtificialOwl merged commit 5920120 into master Feb 12, 2024
@ArtificialOwl ArtificialOwl deleted the test-iappconfig branch February 12, 2024 19:09
@blizzz blizzz mentioned this pull request Mar 5, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@skjnldsv skjnldsv skjnldsv left review comments

@come-nc come-nc come-nc approved these changes

@nickvergessen nickvergessen Awaiting requested review from nickvergessen

+1 more reviewer

@emoral435 emoral435 emoral435 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

3. to review Waiting for reviews

Projects

None yet

Milestone

Nextcloud 29

Development

Successfully merging this pull request may close these issues.

6 participants

@ArtificialOwl @nickvergessen @skjnldsv @come-nc @emoral435