Skip to content

Conversation

@marcelklehr
Copy link
Member

Signed-off-by: Marcel Klehr [email protected]

Signed-off-by: Marcel Klehr <[email protected]>
@codecov-commenter
Copy link

codecov-commenter commented Mar 25, 2022

Codecov Report

Merging #1771 (29bcf5c) into master (e70ccbe) will not change coverage.
The diff coverage is n/a.

❗ Current head 29bcf5c differs from pull request most recent head 05b0ae1. Consider uploading reports for the commit 05b0ae1 to get more accurate results

@@            Coverage Diff            @@
##             master    #1771   +/-   ##
=========================================
  Coverage     46.10%   46.10%           
  Complexity     1429     1429           
=========================================
  Files            96       96           
  Lines          5143     5143           
=========================================
  Hits           2371     2371           
  Misses         2772     2772           

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 e70ccbe...05b0ae1. Read the comment docs.

marcelklehr and others added 9 commits March 26, 2022 09:59
Signed-off-by: Marcel Klehr <[email protected]>
Signed-off-by: Marcel Klehr <[email protected]>
Signed-off-by: Marcel Klehr <[email protected]>
Signed-off-by: Marcel Klehr <[email protected]>
Signed-off-by: Marcel Klehr <[email protected]>
Signed-off-by: Marcel Klehr <[email protected]>
Signed-off-by: Marcel Klehr <[email protected]>
Signed-off-by: Joas Schilling <[email protected]>
@nickvergessen
Copy link
Member

Can you check if nextcloud/server#31825 fixes your issue?

@marcelklehr
Copy link
Member Author

@nickvergessen
Copy link
Member

So, the reason why it doesn't work is because you are installing an older version first which still claims to support oracle.
Only the update test fails, normal unit tests are fine as there you directly install the new version which does not claim to support oracle.

So this is only a theoretical problem:

  1. When you publish the fixed version without Oracle support, people can not install the older one anymore as they will be offered the newer one
  2. When Add missing doc changes and limit string length 4000 to new columns server#31825 is merged, people that have have a old version installed can update just fine as the added check is skipped when the column already existed
  3. When Add missing doc changes and limit string length 4000 to new columns server#31825 is merged new installs will work fine as the check is skipped when oracle is not supported

So to fix it backport e70ccbe to stable stable3 and stable4 and make a release, then your CI job will pass, but as per above no user should be affected

@nickvergessen
Copy link
Member

Btw I saw you are having lots of actions. You are currently running every test twice, once for pushing the commit to the branch and once for the pull request.
I recommend limiting pushes to branches master and stable, like we do in all other apps:
https://github.com/nextcloud/notifications/blob/73e4292311c1f7a2e1590eeb86f78023454e5287/.github/workflows/phpunit.yml#L12-L15

@marcelklehr marcelklehr marked this pull request as ready for review April 5, 2022 14:39
@marcelklehr marcelklehr merged commit 72e2de7 into master Apr 5, 2022
@marcelklehr marcelklehr deleted the test/nc24 branch April 5, 2022 14:40
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.

4 participants