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

Controls: Added metadata to the Controls project to enable publishing to NuGet #16611

Open
wants to merge 53 commits into
base: main
Choose a base branch
from

Conversation

Lamparter
Copy link
Contributor

@Lamparter Lamparter commented Dec 16, 2024

Resolved / Related Issues

Discussed on Discord

Steps used to test these changes

  1. Built the Files controls project
  2. Checked for Roslyn analysers during build
  3. Ensured NuGet package was correctly generated
  4. Inspected NuGet package internals to ensure proper generation and inclusion of XAML resources

WHEN PR IS ACCEPTED (NOT BEFORE)

  • Implement actual secret
  • Set environment to NuGet
  • Remove build on main and PR configurations
  • Add condition for run to only happen on changes made to CurrentVersion.props
  • Replace default GitHub Actions runner token with files-community-bot[bot] token

@Lamparter Lamparter marked this pull request as ready for review December 24, 2024 19:04
@Lamparter Lamparter requested a review from yaira2 December 24, 2024 19:04
@Lamparter
Copy link
Contributor Author

Note for reviewer:
image
The CD fails because there is no such API key provided (please create a NUGET_API_KEY secret in the repository to enable publishing)

@yaira2 yaira2 changed the title Added package metadata to controls project to enable publishing Nuget: Added metadata to the Controls project to enable publishing to Nuget Dec 24, 2024
@Lamparter Lamparter changed the title Nuget: Added metadata to the Controls project to enable publishing to Nuget NuGet: Added metadata to the Controls project to enable publishing to Nuget Dec 24, 2024
@Lamparter Lamparter changed the title NuGet: Added metadata to the Controls project to enable publishing to Nuget NuGet: Added metadata to the Controls project to enable publishing to NuGet Dec 24, 2024
@Lamparter Lamparter requested a review from yaira2 December 24, 2024 20:45
@Lamparter Lamparter marked this pull request as draft January 2, 2025 11:01
@0x5bfa
Copy link
Member

0x5bfa commented Jan 14, 2025

We should do this after v4 or even later. What do you think?

@yaira2
Copy link
Member

yaira2 commented Jan 14, 2025

We can move forward with this as soon as the versioning system is agreed on.

@0x5bfa
Copy link
Member

0x5bfa commented Jan 15, 2025

If you'd like to publish when you publish preview or stable thru Files CD, we can add a parameter text box to input the version. If empty, don't publish; otherwise, try publish with that version string.

@yaira2
Copy link
Member

yaira2 commented Jan 15, 2025

This shouldn't be tied to app releases. We may have a newer control ready for release before an app update, and on the other hand, we may want to release the app without updating the controls. It's best to handle this as a separate action.

@yaira2 yaira2 changed the title NuGet: Added metadata to the Controls project to enable publishing to NuGet Controls: Added metadata to the Controls project to enable publishing to NuGet Feb 9, 2025
@Lamparter Lamparter requested a review from yaira2 February 9, 2025 16:36
@Lamparter
Copy link
Contributor Author

@yaira2 when you approve you'll need to give the bot access to the repo
I think it's the read and write to pull request data permission

Lamparter and others added 19 commits March 25, 2025 18:33
[skip ci]
[skip ci]
[skip ci]
[skip ci]
Co-authored-by: Yair <39923744+yaira2@users.noreply.github.com>
Signed-off-by: Lamparter <71598437+Lamparter@users.noreply.github.com>
[skip ci]

Signed-off-by: Lamparter <71598437+Lamparter@users.noreply.github.com>
Co-authored-by: Yair <39923744+yaira2@users.noreply.github.com>
Signed-off-by: Lamparter <71598437+Lamparter@users.noreply.github.com>
Co-authored-by: Yair <39923744+yaira2@users.noreply.github.com>
Signed-off-by: Lamparter <71598437+Lamparter@users.noreply.github.com>
Co-authored-by: Yair <39923744+yaira2@users.noreply.github.com>
Signed-off-by: Lamparter <71598437+Lamparter@users.noreply.github.com>
Signed-off-by: Lamparter <71598437+Lamparter@users.noreply.github.com>
@Lamparter Lamparter requested a review from yaira2 March 25, 2025 18:34
yaira2
yaira2 previously approved these changes Mar 25, 2025
@yaira2 yaira2 requested a review from 0x5bfa March 25, 2025 18:35
@yaira2 yaira2 added needs - code review and removed changes requested Changes are needed for this pull request labels Mar 25, 2025
Signed-off-by: Lamparter <71598437+Lamparter@users.noreply.github.com>
Copy link
Member

@0x5bfa 0x5bfa left a comment

Choose a reason for hiding this comment

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

Looks good, but I do belive we should not publish yet unless we ultimatelly remove WCT 7 controls (if we do, we should attribute properly) and fiinnsh unfinished controls such as Toolbar and Storage Bar/Ring.

fail-fast: false
env:
WORKING_DIR: '${{ github.workspace }}' # D:\a\Files\Files\
PROPS_PATH: '${{ github.workspace }}\src\Files.App.Controls\CurrentVersion.props'
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
PROPS_PATH: '${{ github.workspace }}\src\Files.App.Controls\CurrentVersion.props'
VERSION_PROPS_PATH: '${{ github.workspace }}\src\Files.App.Controls\CurrentVersion.props'

uses: EndBug/add-and-commit@v9
with:
# The arguments for the `git add` command
# Default: '.'
Copy link
Member

Choose a reason for hiding this comment

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

I dont think we need these comments

Comment on lines +2 to +6
<!-- READ ME BEFORE MODIFYING THIS FILE:
This file is used to track the version of the Files UI Controls package online.
The version is automatically bumped by the 'Bump Files UI Controls' action online.
You can bump the version here in a PR and when it is merged the controls project
will be automatically published to NuGet online. -->
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
<!-- READ ME BEFORE MODIFYING THIS FILE:
This file is used to track the version of the Files UI Controls package online.
The version is automatically bumped by the 'Bump Files UI Controls' action online.
You can bump the version here in a PR and when it is merged the controls project
will be automatically published to NuGet online. -->
<!--
DO NOT MODIFY unless you are knowledgeable about this file:
This file is used to track the version of the Files.App.Controls NuGet package online.
The version is automatically bumped by a GitHub Action online.
-->

Co-authored-by: 0x5BFA <62196528+0x5bfa@users.noreply.github.com>
Signed-off-by: Yair <39923744+yaira2@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants