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

feat: add dotnet8 support #6429

Merged
merged 5 commits into from
Feb 21, 2024
Merged

feat: add dotnet8 support #6429

merged 5 commits into from
Feb 21, 2024

Conversation

Beau-Gosse-dev
Copy link
Contributor

Why is this change necessary?

To add support for .NET 8 Lambdas (managed runtime)

Mandatory Checklist

PRs will only be reviewed after checklist is complete

  • Add input/output type hints to new functions/methods
  • Write design document if needed (Do I need to write a design document?)
  • Write/update unit tests
  • Write/update integration tests
  • Write/update functional tests if needed
  • make pr passes
  • make update-reproducible-reqs if dependencies were changed
  • Write documentation

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@github-actions github-actions bot added area/local/start-api sam local start-api command area/build sam build command area/local/invoke sam local invoke command area/local/start-invoke area/schema JSON schema file labels Dec 11, 2023
@Beau-Gosse-dev
Copy link
Contributor Author

FYI: @mndeveci @hawflau

@Beau-Gosse-dev Beau-Gosse-dev marked this pull request as ready for review December 14, 2023 00:54
@Beau-Gosse-dev Beau-Gosse-dev requested a review from a team as a code owner December 14, 2023 00:54
normj
normj reviewed Dec 14, 2023
View reviewed changes
tests/integration/testdata/buildcmd/Dotnet8/HelloWorld.csproj Outdated
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Amazon.Lambda.Core" Version="1.0.0" />
Copy link
Member

Choose a reason for hiding this comment

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

Probably doesn't matter but this test Lambda project is using old references including the older Serialization package. From a test point of view it probably doesn't matter but it might be worth updating so people don't copy and paste this somewhere else.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated. Also removed the extra usings from the function

@hawflau hawflau changed the base branch from develop to feat/dotnet8 December 14, 2023 19:16
hawflau
hawflau reviewed Dec 15, 2023
View reviewed changes
Copy link
Contributor

@hawflau hawflau left a comment

Choose a reason for hiding this comment

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

Finished 1st round of review. Nothing particular caught my eyes. I will put the changes into our CI to test and see if anything is caught.

@hawflau hawflau mentioned this pull request Dec 16, 2023
Merged
8 tasks
@mndeveci
Copy link
Contributor

mndeveci commented Feb 8, 2024

@Beau-Gosse-dev it seems like this PR now contains changes from develop (git commit hashes changed during rebase I suppose). Can we try to update or re-create PR with only dotnet8 changes?

@Beau-Gosse-dev Beau-Gosse-dev mentioned this pull request Feb 12, 2024
Closed
3 tasks
@Beau-Gosse-dev
Copy link
Contributor Author

@mndeveci sorry about that. I just synced my fork, but by default forks only pull over the develop branch, so it pulled all that in. I force pushed the branch with that merge removed.

I think it's expected that 2 tests are still failing with dotnet8 as an unknown runtime until this PR is merged: aws/aws-lambda-builders#587

@Beau-Gosse-dev Beau-Gosse-dev mentioned this pull request Feb 20, 2024
Closed
@mndeveci mndeveci enabled auto-merge (squash) February 21, 2024 08:01
@mndeveci mndeveci disabled auto-merge February 21, 2024 08:01
@mndeveci mndeveci merged commit 2ff0c7b into aws:feat/dotnet8 Feb 21, 2024
47 of 53 checks passed
@mndeveci mndeveci mentioned this pull request Feb 21, 2024
Merged
hawflau added a commit that referenced this pull request Feb 21, 2024
@mndeveci @Beau-Gosse-dev @hawflau
feat: add dotnet8 support (#6429) (#6723)
e961128 
* feat: add dotnet8 support (#6429)

* feat: add dotnet8 support

* Update dotnet8 test dependencies and remove extra using directives

* Update samcli/lib/utils/preview_runtimes.py

---------

Co-authored-by: Wing Fung Lau <4760060+hawflau@users.noreply.github.com>

* update schema

* add dotnet8 to runtime_supported_by_docker in test utils

---------

Co-authored-by: Beau Gosse <bgosse@amazon.com>
Co-authored-by: Wing Fung Lau <4760060+hawflau@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hawflau hawflau hawflau approved these changes

@normj normj normj left review comments

@mndeveci mndeveci mndeveci approved these changes

Assignees
No one assigned
Labels
area/build sam build command area/local/invoke sam local invoke command area/local/start-api sam local start-api command area/local/start-invoke area/schema JSON schema file pr/external
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Beau-Gosse-dev @mndeveci @normj @hawflau