Skip to content

Conversation

@nickvergessen
Copy link
Member

Signed-off-by: Joas Schilling coding@schilljs.com

Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen nickvergessen added this to the Nextcloud 13 milestone Aug 24, 2017
Signed-off-by: Joas Schilling <coding@schilljs.com>
$initiator,
$shareWith) {
$shareWith,
\DateTime $expiration) {
Copy link
Member

Choose a reason for hiding this comment

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

Should be allowed to be set to null otherwise shares without an expiration date will fail with:

TypeError: Argument 5 passed to OCA\\ShareByMail\\ShareByMailProvider::sendMailNotification() must be an instance of DateTime, null given, called in apps/sharebymail/lib/ShareByMailProvider.php on line 351 at apps/sharebymail/lib/ShareByMailProvider.php#376

Copy link
Member Author

Choose a reason for hiding this comment

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

\DateTime $expiration = null is better?

Copy link
Member

Choose a reason for hiding this comment

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

Correct 👍

@MorrisJobke MorrisJobke added the 3. to review Waiting for reviews label Aug 26, 2017
@codecov
Copy link

codecov bot commented Aug 26, 2017

Codecov Report

Merging #6255 into master will decrease coverage by 22.78%.
The diff coverage is 8.69%.

@@              Coverage Diff              @@
##             master    #6255       +/-   ##
=============================================
- Coverage     53.33%   30.54%   -22.79%     
  Complexity    22480    22480               
=============================================
  Files          1403     1403               
  Lines         86955    86964        +9     
  Branches       1327     1327               
=============================================
- Hits          46377    26566    -19811     
- Misses        40578    60398    +19820
Impacted Files Coverage Δ Complexity Δ
lib/private/Mail/EMailTemplate.php 59.05% <0%> (-1.43%) 42 <1> (+1)
apps/sharebymail/lib/ShareByMailProvider.php 0% <0%> (-59.44%) 84 <0> (ø)
lib/private/Share20/Manager.php 0.33% <0%> (-87.68%) 228 <0> (ø)
core/Controller/LostController.php 79.47% <66.66%> (-0.26%) 32 <0> (ø)
apps/user_ldap/lib/Migration/UUIDFix.php 0% <0%> (-100%) 5% <0%> (ø)
apps/files_versions/lib/AppInfo/Application.php 0% <0%> (-100%) 2% <0%> (ø)
apps/user_ldap/lib/Migration/UUIDFixGroup.php 0% <0%> (-100%) 1% <0%> (ø)
apps/user_ldap/lib/Mapping/GroupMapping.php 0% <0%> (-100%) 1% <0%> (ø)
lib/private/Files/Cache/Wrapper/JailPropagator.php 0% <0%> (-100%) 1% <0%> (ø)
...ivate/Files/Cache/Wrapper/CachePermissionsMask.php 0% <0%> (-100%) 3% <0%> (ø)
... and 376 more

@MorrisJobke MorrisJobke added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Aug 26, 2017
@rullzer rullzer added 2. developing Work in progress and removed 4. to release Ready to be released and/or waiting for tests to finish labels Aug 26, 2017
@rullzer
Copy link
Member

rullzer commented Aug 26, 2017

Failing tests

@MorrisJobke
Copy link
Member

Failing tests

Fixed

@MorrisJobke MorrisJobke added 4. to release Ready to be released and/or waiting for tests to finish and removed 2. developing Work in progress labels Aug 26, 2017
Signed-off-by: Morris Jobke <hey@morrisjobke.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to release Ready to be released and/or waiting for tests to finish

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants