-
Notifications
You must be signed in to change notification settings - Fork 93
Use record to get with
keyword.
#1040
base: main
Are you sure you want to change the base?
Conversation
@@ -60,7 +65,12 @@ public class SubmissionOptions | |||
int? shots = null, | |||
ImmutableDictionary<string, string>? inputParams = null, | |||
string? targetCapability = null) => | |||
new SubmissionOptions( | |||
friendlyName ?? FriendlyName, shots ?? Shots, inputParams ?? InputParams, targetCapability ?? TargetCapability); | |||
this with |
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.
I'd rather remove With
, since it's redundant.
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.
Sounds good, my only thought was to not break existing callers, but happy to go on and pull that method, then.
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.
To unblock e2e, would it be OK to mark this and Default
as obsolete for now?
namespace Microsoft.Quantum.Runtime.Submitters | ||
{ | ||
/// <summary> | ||
/// Options for a job submitted to Azure Quantum. | ||
/// </summary> | ||
public class SubmissionOptions | ||
public record SubmissionOptions |
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.
Maybe it would be better to remove Default
and the private constructor, and instead give each init property a default value?
@@ -5,32 +5,37 @@ | |||
|
|||
#nullable enable | |||
|
|||
namespace System.Runtime.CompilerServices | |||
{ | |||
public class IsExternalInit {} |
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.
As a public type, won't this conflict if another reference uses the same workaround? Can we update Microsoft.Quantum.Runtime.Core target framework to net6.0 instead?
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.
I've tested in other places, and that type seems to be special-cased to not conflict, allowing this as a suggested workaround (as per https://developercommunity.visualstudio.com/t/error-cs0518-predefined-type-systemruntimecompiler/1244809).
Any updates on this? |
This PR modifies the
SubmissionOptions
class to be a record class, allowing other code to use thewith
keyword to modify instances of the class on a field-by-field basis. The existingWith
method can then be refactored to use thewith
keyword.