Is there a reason you checked .sln changes in?

@chingucoding These updates are automatically made by VS when using a "clean" innerloop (so no additional feature areas selected to be built). When switching local branches yesterday and updating my local master branch to this repository's master branch, I had tp first commit these local innerloop sln changes (as otherwise VS complained that I cannot merge because I have local changes not yet committed). Trying to undo these local sln changes does not work as these changes happen automatically by VS each time I open the innerloop sln. As such, I first had to commit these changes to my local master branch in order to be able to bring it up-to-date with the repo.

As such, you are seeing these changes show up here as part of this PR since it is based on my local master branch, yet they are not required for this PR at all.

Why not instead revert the changes to the innerloop.sln file before merging? This is something that I have done a few times already when using the innerloop, it's not a great experience, but it works. It feels like VS will always make changes to the innerloop, commiting them doesn't make much sense. Ignoring innerloop.sln changes is also outlined in the docs right above this section.

I think the underlying issue here is that some PRs commit required changes to the sln file (for example a new project was added to the solution) yet at the time the innerloop solution was not re-opened with a clean InnerLoopAreas file - causing these additional lines to be included as well. For this reason, when I am making such changes (like adding a missing API test project) I always reload the innerloop solution with a clean InnerLoopAreas file and only then commit the changes to the sln file. This seems to work as all unrequired changes are automatically removed by VS and only the actually relevant lines are included in the sln file.

These changes you are seeing here will show up the next time someone modifies the innerloop sln file (for example by adding a project) so it's not like they will only be part of this PR and if I revert them, they will never show up in the future again. However, if I do revert them, I will have to deal with uncommitted innerloop sln file changes again, if I'm not mistaken? And those are an unnecessary pain to work with as outlined above. So...seeing as these changes are not harmful at all (PipsPager, PagerControl,... are all present and correct in the innerloop solution), yet I can now again merge and switch branches just fine, I would welcome it if these automatic changes could be included in this PR here.

Generally speaking, I think we just have to take greater care when reviewing PRs which modify the innerloop sln file that the sln file only contains changes which are still present when using a clean InnerLoopAreas file. If we do that, I am cautiously optimistic changes of this nature will not show up and cause inconveniences any longer.

The last time I worked with the innerloop, when switching feature areas, it would start modifying the file regardless if I commited the innerloop.sln changes or not. So either way you would need to deal with changes to that file since you won't open the innerloop without any feature areas selected. But that might have changed now, not sure if that's still present. Also, assuming the most recent changes to the innerloop file were made as part of adding a new control, the innerloop file almost certainly was opened by visual studio as the add project script opens the innerloop file.

I think we can check in those changes, but next time, it would be better to also mention them in the PR and not "sneak" them in.

/azp run

@Felix-Dev If you have verified the interloop solution that you currently have works, then it sounds fine to check that in. I agree that dealing with VS automatically changing these sln files is a pain. You can run those commands in the future if you want to avoid your PRs including this unrelated file.

The last time I worked with the innerloop, when switching feature areas, it would start modifying the file regardless if I commited the innerloop.sln changes or not. So either way you would need to deal with changes to that file since you won't open the innerloop without any feature areas selected. But that might have changed now, not sure if that's still present. Also, assuming the most recent changes to the innerloop file were made as part of adding a new control, the innerloop file almost certainly was opened by visual studio as the add project script opens the innerloop file.

That is correct, which is why I suggested in the future that such PRs re-open the innerloop sln file with a clean InnerLoopAreas file so that such temporary changes to the sln file will not be committed and as a result won't cause problems with local branch management further down the line.

Observe the following: Adding the PipsPager control feature area to the InnerLoopAreas file causes the following lines to be added to the innerloop sln file:

dev\PipsPager\PipsPager.vcxitems*{ad0c90b0-4845-4d4b-88f1-86f653f8171b}*SharedItemsImports = 4
dev\Repeater\Repeater.vcxitems*{ad0c90b0-4845-4d4b-88f1-86f653f8171b}*SharedItemsImports = 4

dev\PipsPager\APITests\PipsPager_APITests.projitems*{dedc1e4f-cfa5-4443-83eb-e79d425df7e7}*SharedItemsImports = 4
dev\PipsPager\TestUI\PipsPager_TestUI.projitems*{dedc1e4f-cfa5-4443-83eb-e79d425df7e7}*SharedItemsImports = 4

dev\PipsPager\APITests\PipsPager_APITests.projitems*{fbc396f5-26dd-4ca3-981e-c7bc9fea4546}*SharedItemsImports = 4
dev\PipsPager\TestUI\PipsPager_TestUI.projitems*{fbc396f5-26dd-4ca3-981e-c7bc9fea4546}*SharedItemsImports = 4

Now compare that with the lines removed as part of loading the innerloop solution with a clean InnerLoopAreas file (done as part of this PR):

dev\PipsPager\PipsPager.vcxitems*{ad0c90b0-4845-4d4b-88f1-86f653f8171b}*SharedItemsImports = 4
dev\Repeater\Repeater.vcxitems*{ad0c90b0-4845-4d4b-88f1-86f653f8171b}*SharedItemsImports = 4

dev\PipsPager\APITests\PipsPager_APITests.projitems*{dedc1e4f-cfa5-4443-83eb-e79d425df7e7}*SharedItemsImports = 4
dev\PipsPager\TestUI\PipsPager_TestUI.projitems*{dedc1e4f-cfa5-4443-83eb-e79d425df7e7}*SharedItemsImports = 4

dev\PipsPager\APITests\PipsPager_APITests.projitems*{fbc396f5-26dd-4ca3-981e-c7bc9fea4546}*SharedItemsImports = 4
dev\PipsPager\TestUI\PipsPager_TestUI.projitems*{fbc396f5-26dd-4ca3-981e-c7bc9fea4546}*SharedItemsImports = 4

What do we notice? They match, which is why I believe PRs are accidentally adding these lines at a time they have not re-opened the innerloop solution with a clean InnerLoopAreas file. If we make sure that future PRs modifying the innerloop sln file only contain innerloop sln changes which are still present after re-loading the innerloop solution with a clean InnerLoopAreas file then I believe we won't see these troubles occuring again in the future.

I think it makes sense to call this out in the developer guide. Sometimes, changes to the innerloop sln file are unavoidable, such as when adding a new control like the PipsPager, but we should make sure that only the necessary lines are included in the updated sln file. By re-opening the innerloop solution with a clean InnerLoopAreas file before committing the changes made to the innerloop sln file, we make sure that all temporarily added lines are not included any longer. This additional step is a simple one yet, at least for me, is much appreciated as it will make working on WinUI in the innerloop solution quite a bit simpler and I won't have to deal with the issues outlined earlier any longer.

@chingucoding @ranjeshj FYI.

An even better solution would be to know why exactly the innerloop solution keeps changing. If we knew why that happens, it would of course be best to eliminate the reason. In the meantime though, I think it would be best to just point this out in the documentation to make our lives a bit easier here.

An even better solution would be to know why exactly the innerloop solution keeps changing. If we knew why that happens, it would of course be best to eliminate the reason.

@chingucoding I believe the changes keep happening because we do not yet re-load the innerloop solution with a clean InnerLoopsArea file before we make the final innerloop .sln file commit as part of a PR adding necessary changes to the sln file). The lines highlighted directly above (the removed dev\PipsPager\.... lines in the .sln file) are not included by default in the innerloop .sln file to provide a working innerloop experience. They only show up when we include the PipsPager control (feature area) in the innerloop build process.

So...based on all these observations, it is my understanding that when the PipsPager control (or other controls) were added to the solution, the control was, at some point, added to the InnerLoopAreas file to be included in the innerloop build process. Was that done manually or automatically by VS/the used scripts? Unfortunately, I don't know that since I have yet to work with the script which automatically add new projects to the solution. But I am convinced that, as outlined above, we can easily remove these temporary additions to the innerloop sln file by re-loading the innerloop solution with a clean InnerLoopAreas file.

I am not sure how feasible something like a "innerloop cleanup" script would be which would have to be be run before committing changes which contain necessary innerloop sln changes (such as project additions), but one more manual step we can do is to add a note to the InnerLoopAreas file stating roughly: "If required changes were made to the innerloop sln file (such as adding a project), then please re-open the innerloop solution without any feature area selected and only then commit changes made to the innerloop sln file."

If we could somehow automate this process, that would be even better since there's always a chance we might miss out on removing these lines if we have to manually do the innerloop cleanup/review innerloop .sln changes. And once such temporary lines managed to slip through, we will VS removing them automatically again in local builds, which could cause some (minor) inconveniences again.

@chingucoding I assume the temporary changes which happen to the innerloop sln file when we add a feature area to the innerloop build process are required to actually build the selected feature area? If we could stop these temporary changes to the sln file from happening, then this would also solve the issue at hand here. Though I am not knowledgeable enough about VS/MSBUILD to know if if that is an actual option here or not. You probably know more on that to be able to shoot down this theoretical idea 🙂

@ranjeshj Yes, the removed innerloop sln lines are temporary lines which are only added to the solution file when including these optional feature areas in an innerloop build process (PipsPager, PagerControl, ComboBox, NumberBox). The different projects for these controls (test projects, c++/winrt implementation project) are still present and correct in the innerloop solution and removing these lines doesn't break the solution.

There are multiple things here so let me split it up:

1. Why were they included in the first place?

I think the issue was that (for my part with PagerControl) the innerloop builds contained the PagerControl and such added the files. So this was the reason why those at least entered the innerloop.

2. Does the add control script care?
   I would say that the "add control script" does the same as opening the solution yourself. Of course the thing here is that if you add a new control and immediately commit the changes, theoretically it should add the "temporary" entries as those feature areas did not exist before calling the script and as such weren't included.

3. Can we automagically clean innerloop files?
   In theory we could have a script call git reset on those two files so that developers don't have to bother with that. Given that you need git anyway to be able to contribute, this isn't an unreasonable thing to request to exist on developers machines.

4. Stop temporary entries
   That is what I hope we would do soon, those temporary entries are not great to work with. Unfortunately, I have no idea why those exist in the first place or if we could something in the build process to circumvent that. Maybe someone from the team @llongley or @kmahone knows more what we could do.

There are multiple things here so let me split it up:

1. Why were they included in the first place?

I think the issue was that (for my part with PagerControl) the innerloop builds contained the PagerControl and such added the files. So this was the reason why those at least entered the innerloop.

1. Does the add control script care?
   I would say that the "add control script" does the same as opening the solution yourself. Of course the thing here is that if you add a new control and immediately commit the changes, theoretically it should add the "temporary" entries as those feature areas did not exist before calling the script and as such weren't included.
2. Can we automagically clean innerloop files?
   In theory we could have a script call git reset on those two files so that developers don't have to bother with that. Given that you need git anyway to be able to contribute, this isn't an unreasonable thing to request to exist on developers machines.
3. Stop temporary entries
   That is what I hope we would do soon, those temporary entries are not great to work with. Unfortunately, I have no idea why those exist in the first place or if we could something in the build process to circumvent that. Maybe someone from the team @llongley or @kmahone knows more what we could do.

I don't fully understand why VS keeps updating this file sometimes. Even when there are no changes, I notice that VS likes to move things around unfortunately. What if we just add this file to git ignore ?

Wouldn't that result in the file not being there when someone clones the repository?

Also, sometimes we do have to make valid changes to the innerloop sln files (such as when adding new projects). Wouldn't adding this file to gitignore mean these necessary changes then wouldn't show up on devices of other contributors (for example resulting in missing projects)? Developers could locally add these missing projects whenever they need to work on them but that sounds like additional unnecessary work we would burden contributors with (when using the innerloop solution) 🤔