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

OCC-245: Upgrade to latest OC preview to test System.Text.Json #454

Merged
merged 128 commits into from
Jul 28, 2024

Conversation

sarahelsaig
Copy link
Contributor

@sarahelsaig sarahelsaig commented May 30, 2024

OCC-245
Fixes #442

@sarahelsaig
Copy link
Contributor Author

The build timed out again.

I've just found this threadPerHost setting for ZAP, which likely could fix the hang/timeout problem if set to 1. I will add the option to UITT and test it tomorrow.

@Piedone
Copy link
Member

Piedone commented Jul 25, 2024

Hmm, interesting, thanks!

@sarahelsaig
Copy link
Contributor Author

Unfortunately threadPerHost does not work. The only way I was able to mitigate this problem was to reduce the run time of the active scan by setting maxActiveScanDurationInMinutes from 10 to 5.

docs/releases/3.0.0.md Outdated Show resolved Hide resolved
@Piedone
Copy link
Member

Piedone commented Jul 26, 2024

@MikeAlhayek do you want to check this?


public static class OrchardCoreCommerceConfigureOrder
{
public const int Default = 0;
Copy link
Member

Choose a reason for hiding this comment

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

Orchard core defines the default. I think you should use that default and build on the top of it. You should not define your own default just in case OC ever changes the default.

https://github.com/OrchardCMS/OrchardCore/blob/84ae5cb0c8e0ec297a645f32b39695c3ea7003a3/src/OrchardCore/OrchardCore.Abstractions/OrchardCoreConstants.cs#L34

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I kept the constant, but set its value to OrchardCoreConstants.ConfigureOrder.Default so any other constants are defined in relation to the one from OC.

@@ -59,6 +60,8 @@ namespace OrchardCore.Commerce;

public class Startup : StartupBase
{
public override int Order => OrchardCoreCommerceConfigureOrder.Default;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public override int Order => OrchardCoreCommerceConfigureOrder.Default;
public override int Order => OrchardCore.OrchardCoreConstantsConfigureOrder.Default;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see this is not necessary because that's the default value of StartupBase.Order anyway. I've deleted this line instead.

src/Modules/OrchardCore.Commerce/Startup.cs Outdated Show resolved Hide resolved
@Piedone Piedone requested a review from MikeAlhayek July 26, 2024 22:21
@Piedone Piedone merged commit 08018a8 into task/system-text-json-migration Jul 28, 2024
8 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
Development

Successfully merging this pull request may close these issues.

None yet

3 participants