-
Notifications
You must be signed in to change notification settings - Fork 10
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
payload reword #467
payload reword #467
Conversation
Signed-off-by: Lillie Dae <[email protected]>
|
||
class PayloadService : IPayloadService | ||
{ | ||
private readonly IPayloadRepository _payloadRepository; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO move these interfaces to own files appropriate foldered
@@ -251,12 +251,12 @@ private void AddFailure(DicomStatus dicomStatus, StudySerieSopUids uids = defaul | |||
_resultDicomDataset.Add(failedSopSequence); | |||
} | |||
|
|||
if (uids is not null) | |||
if (aids is not null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix typo
.vscode/settings.json
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this file be checked in ?
docker-compose/docker-compose.yml
Outdated
@@ -21,8 +21,8 @@ services: | |||
- 5672:5672 | |||
- 15672:15672 | |||
environment: | |||
RABBITMQ_DEFAULT_USER: "rabbitmq" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need to change this ?
Signed-off-by: Neil South <[email protected]>
Signed-off-by: Neil South <[email protected]>
Signed-off-by: Lillie Dae <[email protected]>
Signed-off-by: Lillie Dae <[email protected]>
@@ -98,7 +98,7 @@ dotnet_naming_style.i_prefix_pascal_case.required_prefix = I | |||
dotnet_naming_style.i_prefix_pascal_case.capitalization = pascal_case | |||
|
|||
# name all constant fields using PascalCase | |||
dotnet_naming_rule.constant_fields_should_be_pascal_case.severity = error | |||
dotnet_naming_rule.constant_fields_should_be_pascal_case.severity = suggestion |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We want to keep it as an error
so all is consistent.
@@ -0,0 +1,74 @@ | |||
using System; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you like to discuss the change request?
@@ -95,6 +96,7 @@ private static void Main(string[] args) | |||
.ConfigureServices((hostContext, services) => | |||
{ | |||
services.AddOptions<InformaticsGatewayConfiguration>().Bind(hostContext.Configuration.GetSection("InformaticsGateway")); | |||
services.AddOptions<HttpPaginationConfiguration>().Bind(hostContext.Configuration.GetSection("InformaticsGateway:httpPaginationSettings")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of registering another configuration object, can we keep it inside the InformaticsGatewayConfiguration
object? and please do update the user guide.
@@ -16,7 +16,7 @@ | |||
|
|||
namespace Monai.Deploy.InformaticsGateway.Common | |||
{ | |||
public record StudySerieSopUids | |||
public record StudySeriesSopAids |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, what does Aids
stand for in this case?
@@ -0,0 +1,119 @@ | |||
using System; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please update user guide with all APIs. Thanks
@@ -35,6 +35,7 @@ | |||
<PrivateAssets>all</PrivateAssets> | |||
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets> | |||
</PackageReference> | |||
<PackageReference Include="FluentAssertions" Version="7.0.0-alpha.1" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use the same version as other projects. Thanks!
Please include a detailed description and update the changelog & user guide. |
Description
Fixes # .
A few sentences describing the changes proposed in this pull request.
Status
Ready/Work in progress/Hold
Types of changes