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

filter .targets / .props earlier #2286

Merged
merged 1 commit into from
Apr 26, 2017
Merged

filter .targets / .props earlier #2286

merged 1 commit into from
Apr 26, 2017

Conversation

0x53A
Copy link
Contributor

@0x53A 0x53A commented Apr 25, 2017

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

@forki
Copy link
Member

forki commented Apr 26, 2017

it's marked WIP?

@0x53A
Copy link
Contributor Author

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 [WIP] filter .targets / .props earlier filter .targets / .props earlier Apr 26, 2017
@forki
Copy link
Member

forki commented Apr 26, 2017

It's called getTartgetFiles but also retrieves props?

@0x53A
Copy link
Contributor Author

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
@forki
Copy link
Member

forki commented Apr 26, 2017

thanks! Looking forward to that PR then ;-)

@matthid
Copy link
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
Copy link
Contributor Author

0x53A commented Apr 26, 2017

The next weekend is supposed to be rainy ...

@matthid
Copy link
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants