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

Boa Constrictor 1.2.1-alpha (nuget-push.yml fix, Gemfile security update) #115

Merged
merged 3 commits into from
Jul 15, 2021

Conversation

pl-aknight
Copy link
Contributor

@pl-aknight pl-aknight commented Jul 15, 2021

Removed INCLUDE_SYMBOLS from nuget-push.yml. According to this comment, this option shouldn't be necessary. We already have the .csproj options set.

Resolved a Gemfile security vulnerability by requiring addressable >= 2.8.0.

This change bumps the Boa Constrictor version to 1.2.1-alpha. It's an alpha release to test the symbols package publishing.

@pl-aknight pl-aknight changed the title Boa Constrictor 1.2.1-alpha (attempted nuget-push.yml fix) Boa Constrictor 1.2.1-alpha (nuget-push.yml fix, Gemfile security update) Jul 15, 2021
@@ -43,6 +43,3 @@ jobs:

# NuGet server uri hosting the packages, defaults to https://api.nuget.org
NUGET_SOURCE: https://api.nuget.org

Copy link

@AraHaan AraHaan Jul 15, 2021

Choose a reason for hiding this comment

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

With this I think you should instead pin the action to my fork of publish-nuget which is Elskom/publish-nuget. But to use it's edits you would have to have it reference the main branch.

However doing that would require using this action first:

      - name: Restore, Build, test, and pack
        uses: Elskom/build-dotnet@main
        with:
          TEST: true # provided you got unit tests you want to run.

This is because during all the patches I implemented together from the original publish-nuget the only way I could properly make it work was to split all the .NET Command line stuff (other then the ones required for dotnet nuget push) into a separate action.

Also note: when you are still using the original action, it will probably try to override the value to include the symbol packages that is set in the csproj so I think the proper fix is to:

  • wait for the PR to fix it is merged in the original action's repository
  • use the fork I mentioned above (which includes other fixes like file globbing support so publishing multiple packages is done in a single invoke of an action)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I'll try this pull request first, and if it doesn't work, I'll try your suggestion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried using your suggestion, but I can't get it working, and I can't figure it out:
https://github.com/q2ebanking/boa-constrictor/runs/3079018775?check_suite_focus=true

Copy link

Choose a reason for hiding this comment

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

I can help you as soon as I get home with a followup pr.

@pl-aknight pl-aknight merged commit e0a36f4 into main Jul 15, 2021
@pl-aknight pl-aknight deleted the release/1.2.1-alpha branch July 15, 2021 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants