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 Directory.Build.props #4828

Merged
merged 2 commits into from
Feb 2, 2024

Conversation

lahma
Copy link
Contributor

@lahma lahma commented Jan 23, 2024

By replacing common.props with Directory.Build.props build will adhere to the more modern way of doing things. Directory.Build.props is inherited to sub-directories and sets the properties. No need for or to remember a separate import.

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.

I had seen build props before, but I didn't know about the convention that it is included in all projects like this without having to explicitly import them. Nice!

@sfmskywalker
Copy link
Member

I'll wait for you to mark the PR as Ready for review before merging.

@lahma lahma force-pushed the directory-build-props branch 3 times, most recently from b4e0252 to 1a19eb4 Compare January 26, 2024 16:48
@lahma
Copy link
Contributor Author

lahma commented Jan 26, 2024

So I went further down the rabbit hole and removed the framework.props and configureawait.props. Now also these properties are handled via the standard Directory.Build.props, inheriting defaults from parents. Now csproj's don't need to have ImplicitUsings or Nullable either, all inherited.

If samples directory would be on root level like src and test, wouldn't need to special case them in src/Directory.Build.props, but maybe a future improvement.

@lahma lahma force-pushed the directory-build-props branch 2 times, most recently from 0d9bcd9 to b449854 Compare January 26, 2024 17:02
@lahma
Copy link
Contributor Author

lahma commented Jan 26, 2024

OK so many files have changed, but hopefully they are quite consistent 🫠

@lahma lahma marked this pull request as ready for review January 26, 2024 17:10
@dwoldo
Copy link

dwoldo commented Jan 29, 2024

This approach is a great way to minimize those "dependency hell" scenarios. We've started using this in our projects, it makes managing versions much easier. Ps. You can override on a project-level by adding VersionOverride parameter:

see: Overriding package versions

@lahma
Copy link
Contributor Author

lahma commented Jan 29, 2024

My hope is to tackle the versioning more deeply with a different PR converting to Central Package Management, which should improve things a bit too.

@sfmskywalker sfmskywalker merged commit 3ff6e3e into elsa-workflows:main Feb 2, 2024
2 checks passed
@sfmskywalker
Copy link
Member

If samples directory would be on root level like src and test, wouldn't need to special case them in src/Directory.Build.props, but maybe a future improvement.

I agree, let's move them to the same level.

@lahma lahma deleted the directory-build-props branch February 2, 2024 10:16
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.

None yet

3 participants