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.

Sign up for GitHub

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

Jump to bottom

Improve doc comments #6284

Merged
merged 2 commits into from
Apr 5, 2021
Merged

Improve doc comments #6284

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0b1bc62
Improve doc comments
gewarren Mar 19, 2021
e1af62b
Add default value back
gewarren Mar 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 49 additions & 29 deletions src/Tasks/ManifestUtil/ApplicationManifest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,11 @@ public ApplicationManifest(string targetFrameworkVersion)
}

/// <summary>
/// Indicates the application configuration file.
/// For a Win32 native manifest, this input is ignored.
/// Gets or sets the application configuration file.
/// </summary>
/// <remarks>
/// For a Win32 native manifest, this input is ignored.
/// </remarks>
[XmlIgnore]
public string ConfigFile
{
Expand All @@ -91,7 +93,7 @@ public override AssemblyReference EntryPoint
}

/// <summary>
/// Specifies the target framework version
/// Gets or sets the target framework version
/// </summary>
[XmlIgnore]
public string TargetFrameworkVersion
Expand All @@ -101,7 +103,7 @@ public string TargetFrameworkVersion
}

/// <summary>
/// Specifies the link to use if there is a failure launching the application.
/// Gets or sets the link to use if there is a failure launching the application.
/// The specified value should be a fully qualified URL or UNC path.
/// </summary>
[XmlIgnore]
Expand Down Expand Up @@ -162,8 +164,9 @@ private bool WinXPRequired
(_fileAssociationList = new FileAssociationCollection(_fileAssociations));

/// <summary>
/// If true, the application will run in IE using WPF's xbap application model.
/// Gets or sets a value that indicates whether the application will run in IE using WPF's XBAP application model.
/// </summary>
/// <value><see langword="true" /> if the application will run in IE using WPF's XBAP application model; otherwise, <see langword="false" />.</value>
[XmlIgnore]
public bool HostInBrowser
{
Expand All @@ -172,11 +175,13 @@ public bool HostInBrowser
}

/// <summary>
/// Indicates the application icon file.
/// Gets or sets the application icon file.
/// </summary>
/// <remarks>
/// The application icon is expressed in the generated application manifest and is used for the start menu and Add/Remove Programs dialog.
/// If this input is not specified then a default icon is used.
/// For a Win32 native manifest, this input is ignored.
/// </summary>
/// </remarks>
[XmlIgnore]
public string IconFile
{
Expand All @@ -185,7 +190,7 @@ public string IconFile
}

/// <summary>
/// Indicates whether the manifest is a ClickOnce application manifest or a native Win32 application manifest.
/// Gets or sets a value that indicates whether the manifest is a ClickOnce application manifest or a native Win32 application manifest.
/// </summary>
[XmlIgnore]
public bool IsClickOnceManifest
Expand All @@ -195,12 +200,14 @@ public bool IsClickOnceManifest
}

/// <summary>
/// Specifies the maximum allowable length of a file path in a ClickOnce application deployment.
/// Gets or sets the maximum allowable length of a file path in a ClickOnce application deployment.
/// </summary>
/// <remarks>
/// If this value is specified, then the length of each file path in the application is checked against this limit.
/// Any items that exceed the limit will result in a warning message.
/// If this input is not specified or is zero, then no checking is performed.
/// For a Win32 native manifest, this input is ignored.
/// </summary>
/// </remarks>
[XmlIgnore]
public int MaxTargetPath
{
Expand Down Expand Up @@ -235,7 +242,7 @@ internal override void OnBeforeSave()
}

/// <summary>
/// Specifies a textual description for the OS dependency.
/// Gets or sets a textual description for the OS dependency.
/// </summary>
[XmlIgnore]
public string OSDescription
Expand All @@ -245,7 +252,7 @@ public string OSDescription
}

/// <summary>
/// Specifies a support URL for the OS dependency.
/// Gets or sets a support URL for the OS dependency.
/// </summary>
[XmlIgnore]
public string OSSupportUrl
Expand All @@ -255,13 +262,15 @@ public string OSSupportUrl
}

/// <summary>
/// Specifies the minimum required OS version required by the application.
/// An example value is "5.1.2600.0" for Windows XP.
/// If this input is not specified a default value is used.
gewarren marked this conversation as resolved.
Show resolved Hide resolved
/// The default value is the minimum supported OS of the .NET Framework, which is "4.10.0.0" for Windows 98SE.
/// However, if the application contains any native or Reg-Free COM references, then the default will be the Windows XP version.
/// For a Win32 native manifest, this input is ignored.
/// Gets or sets the minimum OS version required by the application.
/// </summary>
/// <remarks>
/// An example value is "5.1.2600.0" for Windows XP.
/// If you don't specify a value, a default value is used.
/// The default value is the minimum supported OS of the .NET Framework, which is "4.10.0.0" for Windows 98 Second Edition.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@danmoseley Should this MSBuild logic be updated since Windows 98 is no longer supported?

Copy link
Member

@danmoseley danmoseley Mar 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

This isn't my code/product. I believe this relates to ClickOnce which I don't think (?) is under active development. So my inclination if I owned this would be to not touch functionality unless I have to. I would just update the comment as seems best. It doesn't seem very consequential if it's not quite right.

Copy link
Member

Choose a reason for hiding this comment

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

Team triage: We've seen some PRs from ClickOnce, so it still seems to be under active development. @sujitnayak and @John-Hart can comment on whether they want to make a change, though I agree it doesn't seem too consequential.

Copy link

Choose a reason for hiding this comment

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

ClickOnce is still supported and in fact, we have updated this recently for .NET Core, but as to which versions of Windows and .NET that are supported or the defaults is more of a ClickOnce Runtime question for either @merriemcgaw or @NikolaMilosavljevic

Copy link
Member

Choose a reason for hiding this comment

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

I stand corrected.

Copy link
Member

Choose a reason for hiding this comment

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

@merriemcgaw, @NikolaMilosavljevic, update?

Copy link
Member

Choose a reason for hiding this comment

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

I don't think we should modify the default supported OS. Let's keep the old value intact. Customers can always modify this on their own.

/// However, if the application contains any native or Reg-Free COM references, then the default is the Windows XP version, which is "5.1.2600.0".
/// For a Win32 native manifest, this input is ignored.
/// </remarks>
[XmlIgnore]
public string OSVersion
{
Expand Down Expand Up @@ -304,10 +313,12 @@ public string OSVersion
}

/// <summary>
/// Specifies the name of the application.
/// Gets or sets the name of the application.
/// </summary>
/// <remarks>
/// If this input is not specified then the Product is not written into the Application Manifest
/// This name is used for the shortcut name on the Start menu and is part of the name that appears in the Add/Remove Programs dialog.
/// </summary>
/// </remarks>
[XmlIgnore]
public string Product
{
Expand All @@ -316,10 +327,12 @@ public string Product
}

/// <summary>
/// Specifies the publisher of the application.
/// If this input is not specified then the Publisher is not written into the Application Manifest
/// This name is used for the folder name on the Start menu and is part of the name that appears in the Add/Remove Programs dialog.
/// Gets or sets the publisher of the application.
/// </summary>
/// <remarks>
/// If this input is not, specified then the Publisher is not written into the Application Manifest
/// This name is used for the folder name on the Start menu and is part of the name that appears in the Add/Remove Programs dialog.
/// </remarks>
[XmlIgnore]
public string Publisher
{
Expand All @@ -328,9 +341,11 @@ public string Publisher
}

/// <summary>
/// Specifies the suite name of the application.
/// This name is used for the sub-folder name on the Start menu (as a child of the publisher)
/// Gets or sets the suite name of the application.
/// </summary>
/// <remarks>
/// This name is used for the sub-folder name on the Start menu (as a child of the publisher)
/// </remarks>
[XmlIgnore]
public string SuiteName
{
Expand All @@ -339,9 +354,11 @@ public string SuiteName
}

/// <summary>
/// Specifies the link that appears in the Add/Remove Programs dialog for the application.
/// The specified value should be a fully qualified URL or UNC path.
/// Gets or sets the link that appears in the Add/Remove Programs dialog for the application.
/// </summary>
/// <remarks>
/// The specified value should be a fully qualified URL or UNC path.
/// </remarks>
[XmlIgnore]
public string SupportUrl
{
Expand All @@ -350,7 +367,7 @@ public string SupportUrl
}

/// <summary>
/// Specifies a trust object defining the application security.
/// Gets or sets a trust object defining the application security.
/// </summary>
[XmlIgnore]
public TrustInfo TrustInfo
Expand All @@ -360,8 +377,11 @@ public TrustInfo TrustInfo
}

/// <summary>
/// If true, the install will use the settings in the application manifest in the trust prompt.
/// Gets or sets a value that indicates whether the install will use the settings in the application manifest in the trust prompt.
/// </summary>
/// <value>
/// <see langword="true" /> to use the settings in the application manifest in the trust prompt; otherwise, <see langword="false" />.
/// </value>
[XmlIgnore]
public bool UseApplicationTrust
{
Expand Down