These are chat archives for Microsoft/CodeContracts

9th
Jul 2015
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 00:51
Another git-related question: did you face an issue when some file line endings are automatically converted and some file appears that it has changes
tried git reset --hard, git checkout -- filename, but in all those cases file stays the same with line ending changes...
and I can't discard them
And I can't even stash the change, they're still applied to one file..
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 00:59
I was able to fix this by commenting out for a moment * text=auto in the .gitattributes file.
The whole article is a good read, but that section in particular will likely resolve your issue. You may need to do a single commit if the file is in the repo with the wrong line endings.
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 02:31
@Zoltu Thanks a lot! :+1:
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 03:07
Please take a look at #97, thanks
@hubuk , if you can test this PR I would be really appreciated.
this PR should fix an issue with slow connection to the localdb on your build server.
Sam Harwell
@sharwell
Jul 09 2015 03:47
@SergeyTeplyakov the reformat in the middle of the pull request made it where the github diff (the Files tab of the pull request) won't show the diff for CacheManager.
I left my comment on the specific commit instead
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 03:54
Yeah, I know but Its just impossible to made any reasonable change like that with old code style. And creating 3 PRs instead of one is just too much from my perspective for such a simple (relatevely) fix.
Ok, now I've got why I stayed with .NET 4.0 implementation. Actually Clousot.Cache is 4.0, so I've decided to stay with 4.0 implementation
switch to async implementation for the main method.
Sam Harwell
@sharwell
Jul 09 2015 03:57
Yes, one of the files can use new pattern and one cannot
The formatting issue will be dealt with soon. After we release, we can deal with the other open pull requests (in particular the ones from @tom-englert), and then we can reformat everything.
I'm out for the night, ttyl :)
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 04:01
ok. ttyl
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 16:00
@sharwell I've added another comment on my PR about unboserved exceptions.
Sam Harwell
@sharwell
Jul 09 2015 16:27
@SergeyTeplyakov We should avoid unobserved exceptions by writing tasks that do not fault, not by taking an otherwise correct and succinct method and writing it using old patterns that are difficult to reason about.
The short-form async method I wrote will not fault (it will return null instead). Now if you update CreateAndCheckAsync to also not fault, then we are completely covered and the code can stay short. And if for some reason an implementation of CreateAndCheckAsync does fault, it's no big deal.
Eventually we can add logging attached to UnobservedException, but it's probably not critical yet.
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 16:34
CreateAndCheckAsync could fault just because of polymorphic behavior. We just can't guarantee that every implementation of the IClousotCacheFactory.Create will never fail.

Eventually we can add logging attached to UnobservedException, but it's probably not critical yet.

I would say that the code should do the best (if possible) to avoid UnobservedExceptions because in many cases they means a bugs in the code.
In our current codebase we're crashing the app manually when UnobservedException is happening, just because we don't know is the state of the world is still correct or not.

Sam Harwell
@sharwell
Jul 09 2015 16:36
Correct, that's what UnobservedException is for. At the top level, we documented the correct behavior for failure as returning null. If an implementation of CreateAndCheckAsync violates that, we get an unobserved exception, but CreateCacheAccessorAsync will still return the correct answer.
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 16:36
ok.. I'll add small change to avoid UE
Sam Harwell
@sharwell
Jul 09 2015 16:36
hang on
If you aren't handling the exception, do not add code to avoid the UE
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 16:37
the problem is that we know that UnobservedException could happen here and we know that this is not a bug in the code and not a system failure. That's why we should not allow them in the first place.
We are handling the exceptions. Just by skipping faulted tasks.
Sam Harwell
@sharwell
Jul 09 2015 16:38
At least move the handling up to CreateAndCheckAsync
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 16:39
This is just another way of doing this. Right? :) We're looking for the first successfully completed task, ignoring ones that fail. We're not handling them, but this is semantically equivalent. This code is saying to me: "connect to the cache and please ignore the factory if you can't connect to it"
As I mentioned: I don't think this is a good solution. Just because in this case every client of the CreateAndCheckAsync would not be able to handle the exceptions. And this is not true. Our new method is a bit crazy and wants to swallow or ignore any exceptions, but I can't say that this is true of all other clients..
Sam Harwell
@sharwell
Jul 09 2015 16:40

Just because in this case every client of the CreateAndCheckAsync would not be able to handle the exceptions.

This is not correct. One sec.

Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 16:40
Ok. I have an idea
the method should be TryCreateAndCheckAsync. This will solve the problem

This is not correct. One sec.

Why? :)

Sam Harwell
@sharwell
Jul 09 2015 16:41
hang on
Use this method to observe. The task will still be faulted and can be observed by callers, but it won't end up in an UnobservedException event.
https://github.com/rackerlabs/dotnet-threading/blob/master/Rackspace.Threading/CoreTaskExtensions.cs#L1606-L1631
If you use that code, everything is the same (including the ability to listen to a faulted task) except you won't get the UE
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 16:44
As I said. I know how to observe an exception. The question is: new solution that observes an exceptions would be comparable in complexity with the original one:)
Sam Harwell
@sharwell
Jul 09 2015 16:44
put this in CreateAndCheckAsync for the following reasons:
  1. the code is .NET 4.0, so UE might actually crash
  2. it's a static wrapper method, so there's no way a derived type could break the behavior
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 16:46

the code is .NET 4.0, so UE might actually crash

it's not:) It depends on the app configuration, not on dll where this method was written.

in our case it won't just because the overall app is .net 4.5
Sam Harwell
@sharwell
Jul 09 2015 16:47
right, but the method that's currently async/await is obviously never going to run where UE crashes
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 16:47
we don't know because the behavior depends on the app.config settings
I think we're on the same page that we should avoid UE. That's good.
There is multiple way to do this:)
Sam Harwell
@sharwell
Jul 09 2015 16:49
I am conceding, provided that CreateCacheAccessorAsync is not more complex than it currently is
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 16:49
:))) I think this is the most simplest part of the Clousot codebase:)
Sam Harwell
@sharwell
Jul 09 2015 16:50
And now we're all feeling the pain from it.
Simplicity is our gift to future maintainers :D
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 16:51
the overall code quality is very low (from low level methods to the app design) and overall complexity is very high. I always thinking about the concequences of the solution. If the solution is relatevely complex but local, this is one thing. If the solution is complex and it is spread around the world - this is a problem.
If the code is well documented with clear description of the runtime behavior and intetion it would not be a problem to understand it. If the intention is not clear and no one actually know what it was - it is a problem
and from my perspective, solution that leaves UE silently is much worse from maintenance perspective:)
Sam Harwell
@sharwell
Jul 09 2015 16:55
CreateAndCheckAsync is the right place to avoid it. And even to log it if you want. Plus there you only have to deal with one factory result, not keeping track of all of them and also trying to complete the returned task as early as possible.
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 16:55
and, for instance, using TryCreateCacheAccessorAsync is worse from maintenance perspective as well. I can easily imagine that someone else will start using this method in another context and will ignore an exceptions silently. This is bad from maintenance perspective because could lead to subtle undesirable behavior.
Sam Harwell
@sharwell
Jul 09 2015 16:56
Where did TryCreateCacheAccessorAsync come from?
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 16:56
The problem with swallowing exceptions from low-level methods (or even tracing them) that this method just don't have appropriate context to understand is it desirable or not.
Sam Harwell
@sharwell
Jul 09 2015 16:56
ObserveExceptions does not swallow them
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 16:56
If CreateCacheAccessorAsync will swallow an exception the name should reflect this
I see.
Sam Harwell
@sharwell
Jul 09 2015 16:57
ObserveExceptions passes them through. It is identical to what you have now aside from not producing UE.
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 16:57
Yep. You're right.
Sam Harwell
@sharwell
Jul 09 2015 16:57
(I wrote the library containing that method BTW)
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 16:57
Yeah. I got it:)
Sam Harwell
@sharwell
Jul 09 2015 16:57
lol
If you actually want to handle an exception, there is a Catch<TException> method for that
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 16:59
:)
I'll observe the exception manually, without introducing helper methods
ok?
just because we don't have enough tasks here and this will just complicates the solution without clear benefits right now.
Sam Harwell
@sharwell
Jul 09 2015 17:00
I prefer the helper for the following reasons:
  1. Most people get this wrong
  2. Every time you observe exceptions + pass through, this is how to do it
also it's long. the separate method allows CreateAndCheckAsync to focus on what matters (i.e. what makes it a unique method)
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 17:01
I agree. But this is just unneccessary complexity
we don't have utility methods
I'll put a comment in the code an into the documentation.
We have few places in the code. To introduce helper we need to extract separate utility project.
I don't think this will pays off
Sam Harwell
@sharwell
Jul 09 2015 17:03
Why would you need a new project?
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 17:03
Because adding this utility method to Clousot.Cache makes no sense
Sam Harwell
@sharwell
Jul 09 2015 17:04
what about adding a dependency on Rackspace.Threading? It's a ~16kb library
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 17:04
I don't want to:)
Why for this simple fix to add another dependency?
this one line of code with clear comment. It just won't payed off:)
Sam Harwell
@sharwell
Jul 09 2015 17:04
I generally wouldn't, but if you're talking about another project...
I'll look when you update it
I try to keep the barrier for copying the helper in very low by including complete XML documentation for it
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 17:07
Adding the utility to wrong project doing them non-discoverable
I know. But as I mentioned. All utilities are helpful in certain curcumstances. They should be valuable for the project and discoverable.
and in our case it is not useful right now and we just don't know would they be ever useful
From my perspective this could lead to premature generalization issue....
We can and should evolve the design, focusing on the important stuff...
Sam Harwell
@sharwell
Jul 09 2015 17:12
I commented on GitHub to summarize my takeaway from this discussion (so it's not lost)
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 17:12
Understood your opinion. But I'll add this observation to the CacheManager.
Just because it is high level policy required by CacheManager, and should not be provided by CreateAndCheckAsync method.
Adding this to CreateAndCheckAsync IMO violates principle of the least surprise. No one expect that the task provided by the factory would have such kind of behavior. If you need this behavior - feel free to add it by your own. And I think this will simplify future migration to the library based solution...
Sam Harwell
@sharwell
Jul 09 2015 17:15

No one expect that the task provided by the factory would have such kind of behavior.

what kind of behavior?

Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 17:15
Observation of the exception
Factory method should return a task should not subscribe to it to avoid UE
this is a responsibility of the client
I've pushed the change to PR
Sam Harwell
@sharwell
Jul 09 2015 17:18
What's with the merge in there?
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 17:19
I've merged Microsoft/master....
Sam Harwell
@sharwell
Jul 09 2015 17:20
No, it merged #81
But this is supposed to replace #81.
Do you have Git Extensions by any chance? Highly recommended
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 17:21
I'm usig GitHub for windows. It it lacks tree visualization
I'll clean this up and force push to this branch
Sam Harwell
@sharwell
Jul 09 2015 17:22
:memo: GH4W is pretty, but not functional
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 17:22
yep. I'm using powershell-based cmd for most of my operations...
Sam Harwell
@sharwell
Jul 09 2015 17:27
I kept a copy of your branch here locally so I can verify that the result of the force push is correct (i.e. matches what you currently have up)
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 17:28
ok. Thanks a lot!
Sam Harwell
@sharwell
Jul 09 2015 17:28
let me know when you want me to look again
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 17:28
My steps would be: switch to commit before merge, cherry pick latest one, force push.
sounds good?
Sam Harwell
@sharwell
Jul 09 2015 17:29
I would interactive rebase bugs/CCCHeckCaching onto 29aa333133d32f411680a4b3e8fc7064b0c3b21f, and skip the first commit but keeppick the second (the merge commit would be automatically skipped)
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 17:31
ok. Sounds good too.
Sam Harwell
@sharwell
Jul 09 2015 17:32
The comment inside the foreach loop in ObservePotentialExceptions isn't necessary IMO. Currently it's incomplete, but rather than complete it I would just remove it.
wait until after the rebase to fix that obviously
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 17:36
ok. Sounds good. I'm testing this fix on the configuration with additional cache configurations when only one of them is valid...
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 18:03
Ok. It seems that my fix is not 100% accurate due to DBConnection pooling...
checking...
Sam Harwell
@sharwell
Jul 09 2015 18:04
this is the original fix, unrelated to the async work right?
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 18:05
I've added 7 wrong connection strings and the one that is correct (and correct one is the last one)... and it takes 20 seconds to create the connection...
Sam Harwell
@sharwell
Jul 09 2015 18:08
are the connection strings hard-coded, or can users correct it if there is a problem?
how many connection strings are we actually including by default? I thought just 2?
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 18:10
There is a MinPoolSize property, it is 0 by default
will play with this value
Sam Harwell
@sharwell
Jul 09 2015 18:11
If there are only 2 connection strings and users can modify them if there is a problem, then documenting the workaround might be suitable for a first release (and also using the async creation pattern). One solution for future could be using ESENT instead, but that would definitely be future.
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 18:11
connection strings are not really hardcoded...
connection strings are created using the builder
I'll just pass 2 as a MinPoolSize and will add a comment
we don't need more than too for a moment, and we definitely should look into appropriate solution
Tested: setting MinPoolSize solves the problem
Sam Harwell
@sharwell
Jul 09 2015 18:36
:+1:
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 18:47
I'll update the PR shortly.
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 20:30
Updated PR
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 21:10
This one is critical for my team and basically, I've grabbed the changes that we made locally for our team
but the fix is general and new approach is widely used
@sharwell I've made last PR required (IMO) to the VS2015 support release - #99
I need to understand more clearly how to sync my local repo with original onel....
The only concern that I still see 4 irrelevant commits...
Sam Harwell
@sharwell
Jul 09 2015 21:12
Want me to update it?
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 21:13
no:)) I want to learn how to do this properly:)
What I did is: merged my origin/master from Microsoft/master. Forked from origin/master to feature branch, made a fix there and published it
Sam Harwell
@sharwell
Jul 09 2015 21:14
Your local master branch is not in sync with Microsoft/master. In a consistent state, your master branch should always be able to be fast-forwarded to Microsoft/master.
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 21:15
can I just substitute a head of my master with Microsoft/master somehow?
Sam Harwell
@sharwell
Jul 09 2015 21:15
you can checkout your master branch and reset --hard to the Microsoft/master branch
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 21:16
because every time I merge Microsoft/master 'recursive' strategy was used...
Sam Harwell
@sharwell
Jul 09 2015 21:16
I do this with Git Extensions, not sure what the command is for command line
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 21:16
understood
oh..
installing Git Extensions... :)))
Sam Harwell
@sharwell
Jul 09 2015 21:19
blob
Sam Harwell
@sharwell
Jul 09 2015 21:27
:+1: Your master branch is in the correct place now
Would you like the Git Extensions steps to correct #99?
Hmm. Seems you closed it and deleted the branch. :worried:
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 21:33
yep, I fixed my master
#100 is there
now this issue should not happen any more:)
I already fix my origin/master when you suggested a fix:)
Sam Harwell
@sharwell
Jul 09 2015 21:34
You're going to like this UI
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 21:34
I've installed Git Extensions will use it in the future! Thanks for an advice!
I used SourceTree, then switched to GitHub for Windows
and always used command line tools for merging:)
Sam Harwell
@sharwell
Jul 09 2015 21:35
Git Extensions does almost everything the command line can do, only in a way that makes it very hard to make mistakes. SourceTree and GH4W aren't even close on this front.
commented on #100
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 21:35
ok. Now we're ready to create (at least internal) release of this stuff !!!
Sam Harwell
@sharwell
Jul 09 2015 21:36
internal community preview
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 21:36
I talked to Mike, he will do all the stuff for signing msi by MS creds...
community preview sounds awesome!
Sam Harwell
@sharwell
Jul 09 2015 21:37
I recommend running the build and making a commit if it changes any version numbers. Tag this commit in Git and mark it as a Pre-Release on GitHub. It will look like this.
Then attach the binaries from the signed build to that prerelease. You can see how we attached files to that ANTLR release (the two .jar files in the Downloads section under the release notes).
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 21:38
Yep. There would be a changes based on version number!
I thought to attach not signed build into the github page and use it as a community preview for validation
then Mike will sign (and it will take some time) and then I can create another final release stuff on the github and Mike will upload msi's to gallery
Sam Harwell
@sharwell
Jul 09 2015 21:40
Just make sure all the builds are run from the same tagged commit
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 21:41
ok.
I'll create a PR for the versioning change
Sam Harwell
@sharwell
Jul 09 2015 21:42
Seems like you could do this:
  1. Run the build
  2. Post the binaries to GitHub pre-release notes
  3. Submit the same binaries for signing
  4. When signing is complete, replace binaries in pre-release notes with the signed copies of the same binaries. For the VSIX, when signing is complete just remove the VSIX from the release notes and upload to gallery instead.
If there are NuGet packages that need to be signed, make sure to not upload those to NuGet until the signing is complete (NuGet doesn't allow you to overwrite a build with a new copy of the same version).
Sam Harwell
@sharwell
Jul 09 2015 21:47
:memo: There is only one VSIX for the editor extensions now, but there are two extensions in the gallery. The source.extension.vsixmanifest currently in use specifies the ID of Code Contracts Editor Extensions VS2010 in the gallery. If you instead want to replace Code Contracts Editor Extensions VS2012, you'll need to change the ID in source.extension.vsixmanifest.
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 21:47
Is it possible just to change the name in the gallery?
either way we need this:)
so I don't have an access to signing, VS gallary and nuget. I'll ask Mike to do all of this
Sam Harwell
@sharwell
Jul 09 2015 21:48
The name will be pulled from the manifest when you upload the new VSIX.
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 21:48
So should we change the name in the manifest? or the name is correct already? (without 2010 in it)
Sam Harwell
@sharwell
Jul 09 2015 21:49
I already fixed the name. When you upload the new VSIX, the extension will appear in the gallery as "Code Contracts Editor Extensions".
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 21:49
sounds good!
Sam Harwell
@sharwell
Jul 09 2015 21:49
The ID, however, determines who gets the automatic upgrade notifications.
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 21:49
ok
Sam Harwell
@sharwell
Jul 09 2015 21:50
I'm about to send an important PR related to this where I'll explain my reasoning
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 21:50
ok
Should I add release notes somewhere in the source tree? Like in the Microsoft.Contracts.nuspec ? or something?
Sam Harwell
@sharwell
Jul 09 2015 21:55
I generally place my release notes on the GitHub releases page.
Then in my nuspec I link to that page
I sent #101 with the change I mentioned.
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 21:57
Merged!
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 22:06
Quick question regarding editor extension...
is the buildCC.bat will build it?
Sam Harwell
@sharwell
Jul 09 2015 22:07
@SergeyTeplyakov If #84 and #85 can be merged and nothing breaks, I think you should merge them. I know these improvements are very important to at least one contributor and I'm not sure how long it will be before we get another release out. If you merge them, I will file issues to review any remaining issues I had.
One sec, checking
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 22:07
Actually they could break many customers...
They've changed the assummptions of the code, so the behaviour could be different
For instance, if those PR's change at least one contract for at least one interface that was implemented by the customer, resulting behavior could be changed
Sam Harwell
@sharwell
Jul 09 2015 22:09
My instinct is definitely that direction (i.e. waiting for next release)
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 22:10
I'm chatting with Mike about #84 and #8
Sam Harwell
@sharwell
Jul 09 2015 22:11
We should figure out exactly when "next release" is and let Tom know. One option for #85 is merging a "safe subset" which includes the items we can verify are obviously correct.
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 22:12
Mike is ok to accept them.
I'll accept #85 right now
Sam Harwell
@sharwell
Jul 09 2015 22:15
I'll file my review issues for it then :smile:
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 22:15
ok. Mike said that we can accept both
#84 can't be merged automaticlly...
and about my question regarding editor extension...
Sam Harwell
@sharwell
Jul 09 2015 22:17
I'm checking on the build
you use buildCC <version> right?
That does not build the extension, but I can make it build the extension
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 22:19
ok. I can build the extension in some other way it is not a big deal
just tell me how? From VS? or maybe there is already .bat file to do this
yep. I'm using buildCC version
Sam Harwell
@sharwell
Jul 09 2015 22:19
#102
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 22:19
whatever is easier to you
You're awesome!
Sam Harwell
@sharwell
Jul 09 2015 22:21
#84 had a merge conflict that was introduced when #85 was merged
I'm looking at it now
Oh that was easy to fix
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 22:22
:)
Sam Harwell
@sharwell
Jul 09 2015 22:22
one sec
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 22:22
sure
I'll update nuspec for the release
Sam Harwell
@sharwell
Jul 09 2015 22:23
ok, I can't push the fixed branch to Microsoft/CodeContracts
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 22:24
hm...
Sam Harwell
@sharwell
Jul 09 2015 22:24
(meaning I can't actually do the manual merge), but I can push a new branch to your fork and you can push that branch to Microsoft/master
or I could push to your fork's master branch
and you push that master up to Microsoft/master
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 22:25
BTW, I'm sure that the next release of this tool would be very soon. I think we would have bug fixes for Roslyn anyway...
So we can postpone this or I can push from my master to Microsoft/master
Sam Harwell
@sharwell
Jul 09 2015 22:26
Or I can create a new pull request with the conflicts resolved and you just merge it on GitHub. I would create it in a way that merging it would simultaneously merge #84.
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 22:26
ok
Sam Harwell
@sharwell
Jul 09 2015 22:26
You prefer pushing to master, or new pull request?
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 22:26
PR:)
Sam Harwell
@sharwell
Jul 09 2015 22:27
#103
#84 was automatically closed :+1:
@tom-englert We got your PRs merged! Now that we are caught up you can send smaller ones :laughing:
@SergeyTeplyakov #81 can be closed (superseded by #97)
Sam Harwell
@sharwell
Jul 09 2015 22:33
@SergeyTeplyakov #30 can be closed (fixed by #57)
@SergeyTeplyakov #17 can be closed (fixed by #66)
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 22:42
Done.
I've created a local branch for upcoming release
will create a PR with version update
Ok. I have a build failure for CSharp.Roslyn\ContractAdornments.CSharp.14.csproj
search\ContractAdornments\CSharp.Roslyn\ContractAdornments.CSharp.14.csproj]
ISymbolExtensions.cs(51,51): error CS0012: The type 'System.Collections.Gener
ic.IReadOnlyCollection1<T0>' is defined in an assembly that is not referenced. You must add a reference to assembly 'System.Runtime, Version=4.0.0.0, Culture =neutral, PublicKeyToken=b03f5f7f11d50a3a'. [D:\Sources\CodeContracts_ST\Micros oft.Research\ContractAdornments\CSharp.Roslyn\ContractAdornments.CSharp.14.cspr oj] ISymbolExtensions.cs(51,51): error CS0012: The type 'System.Collections.Gener ic.IEnumerable1<T0>' is defined in an assembly that is not referenced. You mus
t add a reference to assembly 'System.Runtime, Version=4.0.0.0, Culture=neutral
, PublicKeyToken=b03f5f7f11d50a3a'. [D:\Sources\CodeContracts_ST\Microsoft.Rese
arch\ContractAdornments\CSharp.Roslyn\ContractAdornments.CSharp.14.csproj]
ISymbolExtensions.cs(51,51): error CS0012: The type 'System.Collections.IEnum
erable' is defined in an assembly that is not referenced. You must add a refere
nce to assembly 'System.Runtime, Version=4.0.0.0, Culture=neutral, PublicKeyTok
en=b03f5f7f11d50a3a'. [D:\Sources\CodeContracts_ST\Microsoft.Research\ContractA
dornments\CSharp.Roslyn\ContractAdornments.CSharp.14.csproj]
ISymbolExtensions.cs(51,51): error CS0012: The type 'System.Collections.IList
' is defined in an assembly that is not referenced. You must add a reference to
assembly 'System.Runtime, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03
f5f7f11d50a3a'. [D:\Sources\CodeContracts_ST\Microsoft.Research\ContractAdornme
nts\CSharp.Roslyn\ContractAdornments.CSharp.14.csproj]
ISymbolExtensions.cs(51,51): error CS0012: The type 'System.Collections.IColl
ection' is defined in an assembly that is not referenced. You must add a refere
nce to assembly 'System.Runtime, Version=4.0.0.0, Culture=neutral, PublicKeyTok
en=b03f5f7f11d50a3a'. [D:\Sources\CodeContracts_ST\Microsoft.Research\ContractA
dornments\CSharp.Roslyn\ContractAdornments.CSharp.14.csproj]
ISymbolExtensions.cs(51,51): error CS0012: The type 'System.Collections.IStru
cturalComparable' is defined in an assembly that is not referenced. You must ad
d a reference to assembly 'System.Runtime, Version=4.0.0.0, Culture=neutral, Pu
blicKeyToken=b03f5f7f11d50a3a'. [D:\Sources\CodeContracts_ST\Microsoft.Research
\ContractAdornments\CSharp.Roslyn\ContractAdornments.CSharp.14.csproj]
ISymbolExtensions.cs(51,51): error CS0012: The type 'System.Collections.IStru
cturalEquatable' is defined in an assembly that is not referenced. You must add
a reference to assembly 'System.Runtime, Version=4.0.0.0, Culture=neutral, Pub
licKeyToken=b03f5f7f11d50a3a'. [D:\Sources\CodeContracts_ST\Microsoft.Research\
ContractAdornments\CSharp.Roslyn\ContractAdornments.CSharp.14.csproj]
Intellisense\IntellisenseContractsHelper.cs(31,27): error CS0012: The type 'S
ystem.IDisposable' is defined in an assembly that is not referenced. You must a
dd a reference to assembly 'System.Runtime, Version=4.0.0.0, Culture=neutral, P
ublicKeyToken=b03f5f7f11d50a3a'. [D:\Sources\CodeContracts_ST\Microsoft.Researc
h\ContractAdornments\CSharp.Roslyn\ContractAdornments.CSharp.14.csproj]
Sam Harwell
@sharwell
Jul 09 2015 22:45
With buildCC?
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 22:45
Figuring out, what's going on...
yep. When I'm trying to open ContractAdornments.CSharp.14 I'm getting following error
Sam Harwell
@sharwell
Jul 09 2015 22:47
You have .NET 4.6 installed?
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 22:47
I don't have VS2015 on my machine
"The C# project ... is targeting ".NETFramework, Version=v4.6" whitch is not installed
Sam Harwell
@sharwell
Jul 09 2015 22:47
You need it
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 22:47
I can switch to Framework 4.5
Sam Harwell
@sharwell
Jul 09 2015 22:47
no you can't
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 22:47
Understood
Sam Harwell
@sharwell
Jul 09 2015 22:47
it builds against VS 2015 editor assemblies, which target 4.6. Changing the target framework for ContractAdornments.CSharp.14 will break those references and still won't build.
you don't need Visual Studio 2015 or the VS 2015 SDK installed to build it (nuget magic makes this happen), but there's no getting away from having .NET 4.6 around
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 22:50
ok... Understood
I'll use one of the vm's for this. 5 minues:)
Sam Harwell
@sharwell
Jul 09 2015 22:59
Need anything else from me tonight?
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 23:22
Sorry, was out.
I think the only remaining thing is to accept PR for my version changes...
Sam Harwell
@sharwell
Jul 09 2015 23:28
i'll look back after dinner
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 23:34
Sounds good!
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 23:46
Building on machine with VS2015...
Sergey Teplyakov
@SergeyTeplyakov
Jul 09 2015 23:53
Ok, I'm getting a bunch of errors on the machine with ONLY VS2015 installed. It seems that I need both: VS2013 and VS2015...