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

Use NET 8 SDK artifacts output #4779

Merged
merged 1 commit into from
Jan 15, 2024

Conversation

lahma
Copy link
Contributor

@lahma lahma commented Jan 9, 2024

Use the new NET 8 SDK artifacts output layout feature which consolidates bin and obj folders under root level artifacts directory. Need to have Directory.Build.props to root level for proper configuration (you might want to consider such standard way over common.props too). For a solution with large amount of projects this can help both with I/O performance and cleanup (deleting obj and bin folders as they are in single place).

To remove old cruft, might be good idea to run locally in solution root directory:

Get-ChildItem .\ -include bin,obj -Recurse | foreach ($_) { remove-item $_.fullname -Force -Recurse }

@lahma lahma marked this pull request as ready for review January 9, 2024 16:35
@lahma
Copy link
Contributor Author

lahma commented Jan 9, 2024

Seems that there's no GH Actions workflow for pull requests, I could also recommend using NUKE for build automation 😅

Copy link
Member

@sfmskywalker sfmskywalker left a comment

Choose a reason for hiding this comment

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

Hey there, thanks for the tips and the contribution! I will check it out, as well as NUKE. We are currently using GH Actions, and we should enable it for PR's as well 👍🏻

@sfmskywalker sfmskywalker merged commit 7995b85 into elsa-workflows:main Jan 15, 2024
1 check passed
@lahma lahma deleted the artifacts-output branch January 15, 2024 18:44
@lahma
Copy link
Contributor Author

lahma commented Jan 15, 2024

Thanks for accepting the PR, with such large codebase with multiple projects it's now easier to have all compilation artifacts under single directory.

I would be happy to submit a NUKE proposal for your review, no pressure to accept but would show how things would be different (for better or worse).

@sfmskywalker
Copy link
Member

Wow, I just had a quick glance at NUKE and I eagerly want to see it in action. The ability to express the builde pipeline in C# is very compelling, and it appears to be incredible easy to simply have it execute by GtHub Actions using some attribute.

If you want to submit a proposal, I would be more than happy to review and see it run.

@lahma
Copy link
Contributor Author

lahma commented Jan 15, 2024

Great, I'll try to whip something up, might take a bit though 🙂

@sfmskywalker
Copy link
Member

Awesome, thank you. Of course, no rush :)

@markjones-WK
Copy link

markjones-WK commented Jan 15, 2024

fyi, i just pulled a fresh copy of the source from elsa-core-main and was unable to build as I was getting artifact errors. I deleted the new Directory.Build.props file in the root as restarted VS and was able to build. This may need more testing. I switched to th3 3.0.3 branch that does not have this file, and all is good. It could also be my machine as I am installing .net updates and will be rebooting. Just wanted to post what I observed.

@sfmskywalker
Copy link
Member

Thanks for testing and sharing your findings @markjones-WK. I haven't had a chance yet to try for myself, but will report here my own findings when I have them.

@markjones-WK
Copy link

markjones-WK commented Jan 15, 2024 via email

@sfmskywalker
Copy link
Member

The 3.0.3 branch is for the upcoming patch release and ideally not for larger changes. The main branch is for larger changes and additions from which 3.1 will be released around March.

@lahma
Copy link
Contributor Author

lahma commented Jan 16, 2024

@sfmskywalker can you please revert this for now, I somehow missed the incompatibility between root outputs and ProtoGen tasks, they seem to be quite strict about being under project folder. I'll make amends with a PR to build the PRs and then maybe try this again when CI judge rules the code as compatible...

@markjones-WK
Copy link

The 3.0.3 branch is for the upcoming patch release and ideally not for larger changes. The main branch is for larger changes and additions from which 3.1 will be released around March.

I get it, but isn't the Main branch supposed for the current release branch (3.0.2)? As you mentioned, this PR was not tested and is now asking to be reverted, so if I wanted the released 3.0.2 source code, where would I go to get it?

@sfmskywalker
Copy link
Member

@sfmskywalker can you please revert this for now, I somehow missed the incompatibility between root outputs and ProtoGen tasks, they seem to be quite strict about being under project folder. I'll make amends with a PR to build the PRs and then maybe try this again when CI judge rules the code as compatible...

Of course, no worries. Thank you for having figured that one out 👍🏻

sfmskywalker added a commit that referenced this pull request Jan 16, 2024
@sfmskywalker
Copy link
Member

sfmskywalker commented Jan 16, 2024

@markjones-WK The main branch should contain the latest and greatest changes from which we do minor releases (3.1 being the first upcoming one). I am experimenting to see if we can provided patches (revision releases) via tags only. Not sure if it will work out, but so far it seems OK. If you want to get the source code for the 3.0.2 release, checkout the commit with that tag.

@lahma
Copy link
Contributor Author

lahma commented Feb 4, 2024

I've opened asynkron/protoactor-dotnet#2095 to track the problematic behavior.

@sfmskywalker
Copy link
Member

Thank you! I noticed it has been tagged as a bug by the maintainer 👍🏻

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.

3 participants