Skip to content

[vcpkg] Add changelog generator#15271

Closed
grdowns wants to merge 17 commits intomicrosoft:masterfrom
grdowns:changelog-generator
Closed

[vcpkg] Add changelog generator#15271
grdowns wants to merge 17 commits intomicrosoft:masterfrom
grdowns:changelog-generator

Conversation

@grdowns
Copy link
Contributor

@grdowns grdowns commented Dec 23, 2020

Here's the changelog generator used for the release notes, now ported to PowerShell. I've tested this on arbitrary date ranges, and it seems to work well.

You can use this yourself by running the script .\Get-Changelog.ps1, where it will prompt you for a date range and credentials. There's no logging on it at the moment, so expect to be twiddling your thumbs for a minute or so while it works.

@JackBoosY JackBoosY self-assigned this Dec 24, 2020
@JackBoosY JackBoosY added the category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed label Dec 24, 2020
@JackBoosY JackBoosY requested a review from vicroms December 24, 2020 09:02
Copy link
Member

@vicroms vicroms left a comment

Choose a reason for hiding this comment

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

LGTM but my PowerShell knowledge is lacking 😅

@vicroms
Copy link
Member

vicroms commented Dec 30, 2020

Is it possible to put the credentials in an environment variable to avoid being prompted every time?

@grdowns
Copy link
Contributor Author

grdowns commented Jan 1, 2021

Is it possible to put the credentials in an environment variable to avoid being prompted every time?

I thought about this a bit and explored getting cached credentials out of the git credential manager, or by possibly reading a git-credential file or environment variable.

It seems that the git credentials cache doesn't expose the cached passwords. Understandably it's kept in memory and not exposed.

Reading a git credential file is definitely possible, but I don't see any existing utilities for that. I can try to parse a credential file, but that means adding a parameter set to the options, validating the file, and then manually parsing for the correct credentials. The other point is that keeping your credentials stored in a plain text file (e.g. via git config credential.helper store) is generally not recommended as it's a security vulnerability.

Reading an environment variable is just about as bad as reading a plaintext file (depending who you ask), so I think the final conclusion is to make the facility password-storage agnostic.

My recommendation is to create a PSCredential object in your PowerShell session and supply it as an argument when you run the command. The PSCredential object will be reusable for the entire PowerShell session, too. Here's an example:

PS C:\source\vcpkg> $cred = Get-Credential

cmdlet Get-Credential at command pipeline position 1
Supply values for the following parameters:
Credential
PS C:\source\vcpkg> .\scripts\Get-Changelog.ps1 -Credentials $cred

@grdowns
Copy link
Contributor Author

grdowns commented Jan 2, 2021

Added:

  • Credentials validation
  • Progress bar
  • Prevent changelog file overwriting by creating a new one if it already exists

That'll be about it for this PR. @vicroms feel free to merge when ready 🙂

@grdowns grdowns requested a review from strega-nil January 14, 2021 17:40
Copy link
Contributor

@strega-nil strega-nil left a comment

Choose a reason for hiding this comment

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

You should also change the rest of the file in a similar way; @grdowns are you willing to continue this PR? or should I take it over?

@grdowns
Copy link
Contributor Author

grdowns commented Jan 15, 2021

Thanks for the feedback! I'll get to this soon; let's wait to merge until then.

@ras0219-msft ras0219-msft marked this pull request as draft February 11, 2021 08:55
grdowns and others added 3 commits February 11, 2021 19:34
Co-authored-by: nicole mazzuca <mazzucan@outlook.com>
Co-authored-by: nicole mazzuca <mazzucan@outlook.com>
@grdowns grdowns marked this pull request as ready for review February 11, 2021 22:42
@grdowns
Copy link
Contributor Author

grdowns commented Feb 11, 2021

@strega-nil I committed your changes and fixed a couple things as fallout from the changes (see latest commit). This should be ready to merge upon your review. 🙂

@JackBoosY
Copy link
Contributor

Ping @strega-nil @strega-nil-ms for review this PR.

@PhoebeHui PhoebeHui added the requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. label Jul 21, 2021
@vicroms
Copy link
Member

vicroms commented Jul 26, 2021

I have some changes to clean and push to this PR, I'll try to find some time this week to finally get this merged!

@PhoebeHui
Copy link
Contributor

@vicroms, any updates?

@PhoebeHui PhoebeHui removed the requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. label Sep 14, 2021
@BillyONeal
Copy link
Member

I'm closing this for now. @vicroms Please reopen if you want to continue.

@BillyONeal BillyONeal closed this Nov 22, 2021
@BillyONeal BillyONeal reopened this Feb 18, 2022
@BillyONeal
Copy link
Member

What was in this PR needed sufficient additional work that I now believe it fair to start another PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants