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

CP-40844: Adds download source action that gets the latest source code in the production stage #3153

Merged
merged 19 commits into from
Oct 30, 2023

Conversation

CitrixChris
Copy link
Contributor

… in the production stage.

@danilo-delbusso danilo-delbusso changed the title CP-40844 adds download source action that gets the latest source code… CP-40844: Adds download source action that gets the latest source code in the production stage Jun 6, 2023
Copy link
Member

@danilo-delbusso danilo-delbusso left a comment

Choose a reason for hiding this comment

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

While making the changes for the source info to be fetched as part of the client updates XML, there's some low hanging fruits that can be changed even before that 👇

XenAdmin/Alerts/Types/ClientUpdateAlert.cs Outdated Show resolved Hide resolved
XenAdmin/Alerts/Types/ClientUpdateAlert.cs Outdated Show resolved Hide resolved
XenModel/Actions/Updates/DownloadSourceAction.cs Outdated Show resolved Hide resolved
XenModel/Actions/Updates/DownloadSourceAction.cs Outdated Show resolved Hide resolved
XenModel/Actions/Updates/DownloadSourceAction.cs Outdated Show resolved Hide resolved
@danilo-delbusso danilo-delbusso added the needs updating A reviewer has requested changes label Jun 6, 2023
@@ -31,6 +31,7 @@
using System.Diagnostics;
using System.Reflection;
using System.Resources;
using System.Web.UI;
Copy link
Contributor

Choose a reason for hiding this comment

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

The changes in BrandManager.cs and AssemblyInfo.cs are not needed since we're not introducing any new branded values. If I remember well we said we would be adding two new entries source_url and source_checksum in the XCupdates.xml?

Copy link
Contributor Author

@CitrixChris CitrixChris Jun 12, 2023

Choose a reason for hiding this comment

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

source_url yes but i thought we weren't bothered with the checksum on the source as it is just the source. We can though

@@ -3276,5 +3277,15 @@ private void toolStripMenuItemCfu_Click(object sender, EventArgs e)
{
Updates.CheckForUpdates(true);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Aplogies if I'm missing something, but why have we added two items?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

one to be in the alert notification menu one to be in the help menu

Copy link
Contributor

@kc284 kc284 left a comment

Choose a reason for hiding this comment

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

As above.

@CitrixChris CitrixChris added updated Changes completed, please review and removed needs updating A reviewer has requested changes labels Jun 16, 2023
@kc284 kc284 added the ITE PR should be reviewed within this iteration label Aug 4, 2023
Copy link
Member

@danilo-delbusso danilo-delbusso left a comment

Choose a reason for hiding this comment

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

Gonna leave it at this for now, I'd rather wait for the merge conflicts to be addressed first as they're not super straight forward and I might just be commenting on stuff that's already been fixed on master

XenAdmin/Alerts/Types/ClientUpdateAlert.cs Outdated Show resolved Hide resolved
XenAdmin/Alerts/Types/ClientUpdateAlert.cs Outdated Show resolved Hide resolved
XenAdmin/Alerts/Types/ClientUpdateAlert.cs Outdated Show resolved Hide resolved
XenAdmin/Alerts/Types/ClientUpdateAlert.cs Outdated Show resolved Hide resolved
XenAdmin/Alerts/Types/ClientUpdateAlert.cs Outdated Show resolved Hide resolved
XenAdmin/MainWindow.cs Outdated Show resolved Hide resolved
@danilo-delbusso danilo-delbusso added needs updating A reviewer has requested changes ASAP PR should be reviewed as soon as possible and removed updated Changes completed, please review ITE PR should be reviewed within this iteration labels Aug 10, 2023
@CitrixChris CitrixChris added ITE PR should be reviewed within this iteration and removed ASAP PR should be reviewed as soon as possible labels Aug 16, 2023
@CitrixChris CitrixChris added updated Changes completed, please review and removed needs updating A reviewer has requested changes labels Aug 29, 2023
@kc284 kc284 added needs updating A reviewer has requested changes and removed updated Changes completed, please review labels Sep 4, 2023
@CitrixChris CitrixChris force-pushed the CP-40844-xc-source-download branch 2 times, most recently from ed7b988 to 30f4753 Compare September 6, 2023 14:41
…tead. Simplifies messages for source download and client update.

Signed-off-by: Chris Lancaster <[email protected]>
…ction to handle disposal of FileStream

Signed-off-by: Chris Lancaster <[email protected]>
Signed-off-by: Chris Lancaster <[email protected]>
…a where to save dialog

Signed-off-by: Chris Lancaster <[email protected]>
…matic update checks are turned off. Renames message OUT_OF_DATE_WEBSITE to WEBSITE_DOWNLOADS

Signed-off-by: Chris Lancaster <[email protected]>
Copy link
Contributor

@kc284 kc284 left a comment

Choose a reason for hiding this comment

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

I have made one or two more corrections and some refactorging. This is the commit: kc284@094bd18

If you add me as collaborator to your fork, I can send a PR directly and also add comments to explain the change.

Signed-off-by: Konstantina Chremmou <[email protected]>
Copy link
Contributor

@kc284 kc284 left a comment

Choose a reason for hiding this comment

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

I suggest we squash this when we merge it.

@kc284 kc284 added 1 approval PR has been approved by one reviewer and removed updated Changes completed, please review labels Oct 30, 2023
@danilo-delbusso danilo-delbusso added 2 approvals PR has been approved by two reviewers and removed 1 approval PR has been approved by one reviewer labels Oct 30, 2023
@kc284 kc284 merged commit 9a80dc9 into xenserver:master Oct 30, 2023
1 check passed
@danilo-delbusso danilo-delbusso deleted the CP-40844-xc-source-download branch October 30, 2023 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 approvals PR has been approved by two reviewers ITE PR should be reviewed within this iteration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants