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

filter .targets / .props earlier #2286

Merged
merged 1 commit into from Apr 26, 2017

Conversation

Projects
None yet
3 participants
@0x53A
Contributor

0x53A commented Apr 25, 2017

This works around #2283, see my last two comments there

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Apr 26, 2017

Member

it's marked WIP?

Member

forki commented Apr 26, 2017

it's marked WIP?

@0x53A

This comment has been minimized.

Show comment
Hide comment
@0x53A

0x53A Apr 26, 2017

Contributor

yeah, because i didn't run the tests locally, and wanted to get a second opinion whether that is actually the correct way. If you say it looks good, then let's merge it. =)

cc @matthid

Contributor

0x53A commented Apr 26, 2017

yeah, because i didn't run the tests locally, and wanted to get a second opinion whether that is actually the correct way. If you say it looks good, then let's merge it. =)

cc @matthid

@0x53A 0x53A changed the title from [WIP] filter .targets / .props earlier to filter .targets / .props earlier Apr 26, 2017

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Apr 26, 2017

Member

It's called getTartgetFiles but also retrieves props?

Member

forki commented Apr 26, 2017

It's called getTartgetFiles but also retrieves props?

@0x53A

This comment has been minimized.

Show comment
Hide comment
@0x53A

0x53A Apr 26, 2017

Contributor

The naming is, in general, a bit inconsistent. In the Model there is a single list of files, which is only split later on between .targets and .props. A better name would be something like ImportFiles or BuildFiles, but that is for a later day to refactor.

Contributor

0x53A commented Apr 26, 2017

The naming is, in general, a bit inconsistent. In the Model there is a single list of files, which is only split later on between .targets and .props. A better name would be something like ImportFiles or BuildFiles, but that is for a later day to refactor.

@forki forki merged commit b400a79 into fsprojects:master Apr 26, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Apr 26, 2017

Member

thanks! Looking forward to that PR then ;-)

Member

forki commented Apr 26, 2017

thanks! Looking forward to that PR then ;-)

@matthid

This comment has been minimized.

Show comment
Hide comment
@matthid

matthid Apr 26, 2017

Member

I will look over it on the evening. But if forki wants to release I'm fine with that. It's unlikely worse than what we have (I mean now that we are slowly understanding what we do) ;)

Member

matthid commented Apr 26, 2017

I will look over it on the evening. But if forki wants to release I'm fine with that. It's unlikely worse than what we have (I mean now that we are slowly understanding what we do) ;)

@0x53A

This comment has been minimized.

Show comment
Hide comment
@0x53A

0x53A Apr 26, 2017

Contributor

The next weekend is supposed to be rainy ...

Contributor

0x53A commented Apr 26, 2017

The next weekend is supposed to be rainy ...

@matthid

This comment has been minimized.

Show comment
Hide comment
@matthid

matthid Apr 26, 2017

Member

Yep that PR is fine :)

Member

matthid commented Apr 26, 2017

Yep that PR is fine :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment