Skip to content
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

SC_Response::sendRedirect() transactionid= を画一的に追加せず、一定条件での継承のみとする #922 #960

Merged
merged 8 commits into from
Jul 29, 2024

Conversation

seasoftjapan
Copy link
Contributor

fixed #922

@seasoftjapan seasoftjapan added this to the 2.17.3 milestone Jul 27, 2024
@seasoftjapan seasoftjapan self-assigned this Jul 27, 2024
Copy link

codecov bot commented Jul 27, 2024

Codecov Report

Attention: Patch coverage is 20.00000% with 4 lines in your changes missing coverage. Please review.

Project coverage is 55.63%. Comparing base (3c70f3c) to head (146e122).

Files Patch % Lines
data/class/SC_Response.php 20.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #960      +/-   ##
==========================================
- Coverage   55.72%   55.63%   -0.10%     
==========================================
  Files          75       75              
  Lines        8913     8917       +4     
==========================================
- Hits         4967     4961       -6     
- Misses       3946     3956      +10     
Flag Coverage Δ
tests 55.63% <20.00%> (-0.10%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nanasess デバッグ中に何かの都合(詳細失念)でポート番号を変更していたのですが、改めて試すと :9999 とか、テキトーな番号でもローカルの phpunit は通るようでした。
tests/class/fixtures/server/common.php で putenv('HTTP_URL=http://127.0.0.1:8085/'); しているから、実際のポート番号は何でも良い感じでしょうか。
そうなると、8085 は避けて、元の 8053 にしておくのが適切でしょうか?

Copy link
Contributor

Choose a reason for hiding this comment

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

ここで使用するポート番号はビルトインウェブサーバーが LISTEN する番号ですので、他と被らなければ何でもいいと思います。
8085 でも OK です

Copy link
Contributor Author

Choose a reason for hiding this comment

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

承知いたしました。とりあえず、現状で大丈夫そうですね。
パラレル実行するときに問題となりそうなので、一応 Issue 登録しました。#964

Copy link
Contributor

@nanasess nanasess left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

Choose a reason for hiding this comment

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

ここで使用するポート番号はビルトインウェブサーバーが LISTEN する番号ですので、他と被らなければ何でもいいと思います。
8085 でも OK です

@seasoftjapan seasoftjapan merged commit e2818f4 into EC-CUBE:master Jul 29, 2024
88 of 90 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants