(GH-43) Add Support for running Cake Frosting Projects #46
+673
−111
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ecampidoglio
requested changes
Mar 26, 2024
There was a problem hiding this comment.
Thank you for your contribution. This looks really good—I especially liked how your implementation fits right in with the existing structure and coding style.
There are couple of minor points to address and we're missing the necessary integration tests (see tests.yml)
I understand this review comes more than a year later. If you'd rather not deal with the feedback, I'll be more than happy to take over the PR and address it myself.
ecampidoglio
added
the
waiting for author
|
Thank you for the review. I will make the recommended changes and update the pull request |
|
@louisfischer That sounds great! I wrote a couple of integration tests for Cake Frosting. Would it be OK if I push a commit in this PR branch so you can take a look at the structure? |
|
That sounds great to me. I'll wait to hear from you when it's ready
…On Thu, Mar 28, 2024, 4:21 AM Enrico Campidoglio ***@***.***> wrote:
@louisfischer <https://github.com/louisfischer> That sounds great!
I wrote a couple of integration tests for Cake Frosting. Would it be OK if
I push a commit in this PR branch so you can look at the structure?
—
Reply to this email directly, view it on GitHub
<#46 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAORC4SRR2ZXPSCR2GSIVX3Y2PHJPAVCNFSM6AAAAAAWEVALVOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRUG42DGMZXGE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
ecampidoglio
force-pushed
the
cake-frosting
branch
from
4f0bbac to
d969cff
Compare
|
@louisfischer I rebased your PR on top of |
louisfischer
added a commit
to louisfischer/cake-action
that referenced
this pull request
Apr 14, 2024
louisfischer
added a commit
to louisfischer/cake-action
that referenced
this pull request
Apr 14, 2024
cake.test.ts Reorder imports to previious commit Signed-off-by: Fischer, Louis <louisfischer@gmail.com> Update __tests__/cake.test.ts Co-authored-by: Enrico Campidoglio <enrico.campidoglio@gmail.com> Moved variable per suggestion cake-build#46 (comment) action.ts Reorder imports to previous commit Change the Verbosity Flag to -v to only the .net project references it.
louisfischer
force-pushed
the
cake-frosting
branch
from
0b1eb32 to
a751462
Compare
|
@ecampidoglio Sory for the late fixes. I just realized that that i push all the codes separately. I squashed all the suggestions and fixes into one commit. That should complete it. Is there anyone suggestions you might have found? |
louisfischer
added a commit
to louisfischer/cake-action
that referenced
this pull request
Apr 14, 2024
cake.test.ts Reorder imports to previious commit Signed-off-by: Fischer, Louis <louisfischer@gmail.com> Update __tests__/cake.test.ts Co-authored-by: Enrico Campidoglio <enrico.campidoglio@gmail.com> Moved variable per suggestion cake-build#46 (comment) action.ts Reorder imports to previous commit Change the Verbosity Flag to -v to only the .net project references it. Rebuild the index.js file with changes
louisfischer
force-pushed
the
cake-frosting
branch
from
a751462 to
b1ed0e2
Compare
louisfischer
added a commit
to louisfischer/cake-action
that referenced
this pull request
Apr 14, 2024
cake.test.ts Reorder imports to previious commit Signed-off-by: Fischer, Louis <louisfischer@gmail.com> Update __tests__/cake.test.ts Co-authored-by: Enrico Campidoglio <enrico.campidoglio@gmail.com> Moved variable per suggestion cake-build#46 (comment) action.ts Reorder imports to previous commit Change the Verbosity Flag to -v to only the .net project references it. Rebuild the index.js file with changes Unit Tests fixed
louisfischer
force-pushed
the
cake-frosting
branch
from
b1ed0e2 to
4d9c847
Compare
louisfischer
added a commit
to louisfischer/cake-action
that referenced
this pull request
Apr 14, 2024
cake.test.ts Reorder imports to previious commit Signed-off-by: Fischer, Louis <louisfischer@gmail.com> Update __tests__/cake.test.ts Co-authored-by: Enrico Campidoglio <enrico.campidoglio@gmail.com> Moved variable per suggestion cake-build#46 (comment) action.ts Reorder imports to previous commit Change the Verbosity Flag to -v to only the .net project references it. Rebuild the index.js file with changes Unit Tests fixed Removed verbosity flag
louisfischer
force-pushed
the
cake-frosting
branch
from
4d9c847 to
046d88c
Compare
louisfischer
added a commit
to louisfischer/cake-action
that referenced
this pull request
Apr 15, 2024
cake.test.ts Reorder imports to previious commit Signed-off-by: Fischer, Louis <louisfischer@gmail.com> Update __tests__/cake.test.ts Co-authored-by: Enrico Campidoglio <enrico.campidoglio@gmail.com> Moved variable per suggestion cake-build#46 (comment) action.ts Reorder imports to previous commit Change the Verbosity Flag to -v to only the .net project references it. Rebuild the index.js file with changes Unit Tests fixed Removed verbosity flag Use -- to separate cake-frosting params from dotnet run
louisfischer
force-pushed
the
cake-frosting
branch
from
046d88c to
337a14b
Compare
ecampidoglio
pushed a commit
to louisfischer/cake-action
that referenced
this pull request
Apr 15, 2024
cake.test.ts Reorder imports to previious commit Signed-off-by: Fischer, Louis <louisfischer@gmail.com> Update __tests__/cake.test.ts Co-authored-by: Enrico Campidoglio <enrico.campidoglio@gmail.com> Moved variable per suggestion cake-build#46 (comment) action.ts Reorder imports to previous commit Change the Verbosity Flag to -v to only the .net project references it. Rebuild the index.js file with changes Unit Tests fixed Removed verbosity flag Use -- to separate cake-frosting params from dotnet run
ecampidoglio
force-pushed
the
cake-frosting
branch
from
337a14b to
ebc1515
Compare
ecampidoglio
removed
the
waiting for author
ecampidoglio
pushed a commit
to louisfischer/cake-action
that referenced
this pull request
Apr 15, 2024
cake.test.ts Reorder imports to previious commit Signed-off-by: Fischer, Louis <louisfischer@gmail.com> Update __tests__/cake.test.ts Co-authored-by: Enrico Campidoglio <enrico.campidoglio@gmail.com> Moved variable per suggestion cake-build#46 (comment) action.ts Reorder imports to previous commit Change the Verbosity Flag to -v to only the .net project references it. Rebuild the index.js file with changes Unit Tests fixed Removed verbosity flag Use -- to separate cake-frosting params from dotnet run
ecampidoglio
force-pushed
the
cake-frosting
branch
from
5722d5f to
d6f1335
Compare
ecampidoglio
pushed a commit
to louisfischer/cake-action
that referenced
this pull request
Apr 16, 2024
cake.test.ts Reorder imports to previious commit Signed-off-by: Fischer, Louis <louisfischer@gmail.com> Update __tests__/cake.test.ts Co-authored-by: Enrico Campidoglio <enrico.campidoglio@gmail.com> Moved variable per suggestion cake-build#46 (comment) action.ts Reorder imports to previous commit Change the Verbosity Flag to -v to only the .net project references it. Rebuild the index.js file with changes Unit Tests fixed Removed verbosity flag Use -- to separate cake-frosting params from dotnet run
ecampidoglio
force-pushed
the
cake-frosting
branch
from
055426d to
07804c2
Compare
ecampidoglio
pushed a commit
to louisfischer/cake-action
that referenced
this pull request
Apr 26, 2024
cake.test.ts Reorder imports to previious commit Signed-off-by: Fischer, Louis <louisfischer@gmail.com> Update __tests__/cake.test.ts Co-authored-by: Enrico Campidoglio <enrico.campidoglio@gmail.com> Moved variable per suggestion cake-build#46 (comment) action.ts Reorder imports to previous commit Change the Verbosity Flag to -v to only the .net project references it. Rebuild the index.js file with changes Unit Tests fixed Removed verbosity flag Use -- to separate cake-frosting params from dotnet run
ecampidoglio
force-pushed
the
cake-frosting
branch
from
07804c2 to
ab61dde
Compare
devlead
pushed a commit
to louisfischer/cake-action
that referenced
this pull request
Sep 26, 2024
cake.test.ts Reorder imports to previious commit Signed-off-by: Fischer, Louis <louisfischer@gmail.com> Update __tests__/cake.test.ts Co-authored-by: Enrico Campidoglio <enrico.campidoglio@gmail.com> Moved variable per suggestion cake-build#46 (comment) action.ts Reorder imports to previous commit Change the Verbosity Flag to -v to only the .net project references it. Rebuild the index.js file with changes Unit Tests fixed Removed verbosity flag Use -- to separate cake-frosting params from dotnet run
devlead
force-pushed
the
cake-frosting
branch
from
ab61dde to
5bbad79
Compare
ecampidoglio
force-pushed
the
cake-frosting
branch
2 times, most recently
from
037dd4d to
26c596b
Compare
ecampidoglio
dismissed
devlead’s stale review
The merge-base changed after approval.
ecampidoglio
force-pushed
the
master
branch
from
6ea9cab to
ce69ea7
Compare
ecampidoglio
force-pushed
the
cake-frosting
branch
from
26c596b to
046458f
Compare
ecampidoglio
force-pushed
the
master
branch
from
ce69ea7 to
e313098
Compare
ecampidoglio
force-pushed
the
cake-frosting
branch
from
046458f to
bb07c60
Compare
There was a problem hiding this comment.
@louisfischer Thank you for addressing the feedback. The PR looks good now.
One thing left to do is to add more integration tests to cover all the usage scenarios supported by the Cake Action with Cake Frosting.
Ideally, we want to have feature parity between using a script and a Cake Frosting project, minus the options that don't apply to Cake Frosting, of course (cake-version and cake-bootstrap).
If you prefer, we can add the integration tests ourselves before merging this PR. Please, let us know how you'd like to proceed.
Closed
|
@ecampidoglio if you want to add some additional integration tests. I am not sure when I would be able to get to. I |
It's a good idea to make the discriminator field read-only in a discriminated union to make it more type-safe.
… parameters
This commit introduces a new type to represent the input file called `File`:
```typescript
{
type: string,
path: string
}
```
`File` is a discriminated union type that can either be a `ScriptFile`
or a `ProjectFile`. The `path` field is set to either the value of the
`script-path` or `csproj-path` input parameter, based on what is provided
by the user.
This new abstraction makes is clear that only one type of file
can be executed at any one time by the Cake Action. Also, it allows us to
more clearly express that the `csproj-path` input parameter takes precedence
over the `script-path` parameter.
ecampidoglio
force-pushed
the
cake-frosting
branch
from
bb07c60 to
2596b08
Compare
Previous mentions of 'script file' and 'script arguments' are now called 'build file' and 'build arguments' to include both scripts as well as projects.
ecampidoglio
force-pushed
the
cake-frosting
branch
from
2596b08 to
2321ab4
Compare
ecampidoglio
force-pushed
the
cake-frosting
branch
from
2321ab4 to
abbd3d6
Compare
ecampidoglio
approved these changes
Dec 4, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added support for cake frosting projects
Fixes #43