Skip to content

New colors#37649

Closed
phil-davis wants to merge 9 commits intomasterfrom
new-ci-colors-20200703
Closed

New colors#37649
phil-davis wants to merge 9 commits intomasterfrom
new-ci-colors-20200703

Conversation

@phil-davis
Copy link
Copy Markdown
Contributor

@phil-davis phil-davis commented Jul 3, 2020

Description

PR #37628 gets a weird browser crash in drone CI webUI tests.

I will add the changes there one-by-one to this PR and see when it goes wrong.

Related Issue

How Has This Been Tested?

CI

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:
  • Changelog item, see TEMPLATE

@phil-davis phil-davis self-assigned this Jul 3, 2020
@update-docs
Copy link
Copy Markdown

update-docs bot commented Jul 3, 2020

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@phil-davis phil-davis mentioned this pull request Jul 3, 2020
Closed
11 tasks
@ChrisEdS
Copy link
Copy Markdown

ChrisEdS commented Jul 3, 2020

Thanks for your help @phil-davis

@codecov
Copy link
Copy Markdown

codecov bot commented Jul 3, 2020 •
edited
Loading

Codecov Report

Merging #37649 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #37649   +/-   ##
=========================================
  Coverage     64.74%   64.74%           
  Complexity    19360    19360           
=========================================
  Files          1281     1281           
  Lines         75622    75622           
  Branches       1333     1333           
=========================================
  Hits          48959    48959           
  Misses        26271    26271           
  Partials        392      392
Flag Coverage Δ Complexity Δ
#javascript 54.03% <ø> (ø) 0.00 <ø> (ø)
#phpunit 65.92% <100.00%> (ø) 19360.00 <0.00> (ø)
Impacted Files Coverage Δ Complexity Δ
...ilesharing/templates/settings-personal-general.php 100.00% <ø> (ø) 0.00 <0.00> (ø)
core/templates/layout.base.php 0.00% <ø> (ø) 0.00 <0.00> (ø)
core/templates/layout.guest.php 0.00% <ø> (ø) 0.00 <0.00> (ø)
core/templates/layout.user.php 0.00% <ø> (ø) 0.00 <0.00> (ø)
apps/dav/lib/HookManager.php 53.33% <100.00%> (ø) 17.00 <0.00> (ø)
lib/private/legacy/defaults.php 59.25% <100.00%> (ø) 45.00 <0.00> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aa81463...6b72e23. Read the comment docs.

DeepDiver1975
DeepDiver1975 reviewed Jul 3, 2020
View reviewed changes
apps/dav/lib/HookManager.php
try {
$this->calDav->createCalendar($principal, 'personal', [
'{DAV:}displayname' => $this->l10n->t('Personal'),
'{http://apple.com/ns/ical/}calendar-color' => '#1d2d44']);
Copy link
Copy Markdown
Member

@DeepDiver1975 DeepDiver1975 Jul 3, 2020

Choose a reason for hiding this comment

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

The calendar color corresponds to the owncloud color? Interesting ....

Copy link
Copy Markdown

@ChrisEdS ChrisEdS Jul 3, 2020

Choose a reason for hiding this comment

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

I searched and replaced the previously used color project wide. Must be a hell of a coincidence.

Copy link
Copy Markdown
Contributor Author

@phil-davis phil-davis Jul 3, 2020

Choose a reason for hiding this comment

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

I have no idea about that. I am just trying to find which thing crashes the UI tests.
And I guess that @ChrisEdS did a global search-replace of this color code.

Copy link
Copy Markdown

@ChrisEdS ChrisEdS Jul 3, 2020

Choose a reason for hiding this comment

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

Correct.

@phil-davis
Copy link
Copy Markdown
Contributor Author

phil-davis commented Jul 3, 2020

All the ordinary color code changes pass fine.
I added the new version of core/img/background.jpg - let's see what happens to CI for that.

@phil-davis
Copy link
Copy Markdown
Contributor Author

phil-davis commented Jul 3, 2020

The core/img/background.jpg from the original PR crashes the CI.
The core/img/background.jpg that is 1923x1280 pixels works.

@phil-davis phil-davis mentioned this pull request Jul 3, 2020
Merged
11 tasks
@phil-davis
Copy link
Copy Markdown
Contributor Author

phil-davis commented Jul 3, 2020

See PR #37650 for this code with the commits squashed... for review.

@phil-davis
Copy link
Copy Markdown
Contributor Author

phil-davis commented Jul 4, 2020

Merged in #37650

@phil-davis phil-davis closed this Jul 4, 2020
@phil-davis phil-davis deleted the new-ci-colors-20200703 branch July 4, 2020 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DeepDiver1975 DeepDiver1975 DeepDiver1975 left review comments

+1 more reviewer

@ChrisEdS ChrisEdS ChrisEdS left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@phil-davis phil-davis

Labels

2 - Developing

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@phil-davis @ChrisEdS @DeepDiver1975