These are chat archives for Ruhrpottpatriot/GW2.NET

13th
Sep 2015
shurne
@SamHurne
Sep 13 2015 00:18
Cool stuff. First time using Gitter myself. Haven't even used Slack - we just use google hangouts/chat where I work
shurne
@SamHurne
Sep 13 2015 01:09
Looked at the appveyor build log, looks like it's just missing nuget package restoration - see http://www.appveyor.com/docs/nuget#restoring-nuget-packages-before-build
Steven Liekens
@StevenLiekens
Sep 13 2015 08:37
here's how I solved that for another project
https://github.com/StevenLiekens/TextFx/blob/master/appveyor.yml
before_build:
   - cmd: nuget restore
I'll go ahead and add that now
Steven Liekens
@StevenLiekens
Sep 13 2015 08:43
are you guys good with MSBuild?
I mean the scripting language
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 09:12
I already added NuGet package restore
Ah my bad
I added a appveyor file, but it does overwrite the web config in full. So I have to re-add the package restore. Doing it now
Steven Liekens
@StevenLiekens
Sep 13 2015 09:15
I already did
Ruhrpottpatriot/GW2.NET@2ed9e54
I'm not sure why it doesn't start a new build when I push commits
there's some weirdness going on with the build in general
I'm getting: Found conflicts between different versions of the same dependent assembly that could not be resolved.
Steven Liekens
@StevenLiekens
Sep 13 2015 09:27
ehhh I removed some unused references and that fixes it
specifically System.Runtime
okay quick question
when this is all set up
are we gonna commit directly to the repo on your account, or fork and send pull requests?
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 09:32
You setting was not correct, it has to be
before_build:
- nuget restore
Steven Liekens
@StevenLiekens
Sep 13 2015 09:32
oh sorry :)
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 09:32
fuuu markdown
Steven Liekens
@StevenLiekens
Sep 13 2015 09:32
that's probably why it didn't trigger a build
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 09:33
I also added all the other settings to the appveyor file
And for now let's fork and send pull requests
That's the same way Github Desktop does if you want to merge another branch
Sidenote: A test is failing, therefore the build is failing. Fixing it now
Steven Liekens
@StevenLiekens
Sep 13 2015 09:44
I can see why
class BuildConverter now expects that you pass the response object as object state
but apparently the code doesn't do that
L59 in BuildService.cs
change this.converterForBuild.Convert(dataContract, null)
should be this.converterForBuild.Convert(dataContract, response)
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 09:47
yeah, already did that. Thanks for clarifying it.
Steven Liekens
@StevenLiekens
Sep 13 2015 09:48
sweet
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 09:54
now everything builds. Badge should update in a sec
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 10:36
Yup
Steven Liekens
@StevenLiekens
Sep 13 2015 10:36
the second warning
what makes it not CLS compliant?
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 10:36
lemmy check
btw: Seems build on push is working now
I disabled an increase in build number on pull request so only builds on master and the branches increase the build number
Steven Liekens
@StevenLiekens
Sep 13 2015 10:39
but only when you push
or not
idk
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 10:39
hm
I'll dig into that later
Steven Liekens
@StevenLiekens
Sep 13 2015 10:41
okay one more thing
in appveyor.yml
we need to specify that it should build the Release configuration
otherwise it uses the default configuration from each csproj file
which is always Debug for new projects created in Visual Studio
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 10:43
which platform?
x86?
or any?
Steven Liekens
@StevenLiekens
Sep 13 2015 10:43
AnyCPU
or Any CPU (with space)
I forget when to use which one
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 10:43
I set it in the web config and Export the yml file
Steven Liekens
@StevenLiekens
Sep 13 2015 10:44
I think it's 'Any CPU' because we are building a solution file
it's 'AnyCPU' when you build individual project files
MSBuild kinda sucks like that
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 10:46
funny thing:
Steven Liekens
@StevenLiekens
Sep 13 2015 10:46
anyway there's no reason to target x86 or x64 unless we call into native DLLs
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 10:46
If I set the cls compliance in the assemblyinfo file, we get no warning
Steven Liekens
@StevenLiekens
Sep 13 2015 10:46
-.- but why
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 10:46
if I leave it at the SharedAssemblyInfo we get only the warning in the GW2NET file
Steven Liekens
@StevenLiekens
Sep 13 2015 10:49
do you know why?
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 10:49
sadly.. no
err
Steven Liekens
@StevenLiekens
Sep 13 2015 10:50
ohhh
I do
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 10:50
how can I delete a commit from the history
if you look here
The last commit was done because I forgot a pull
how can I delete it?
Steven Liekens
@StevenLiekens
Sep 13 2015 10:51
git reset --hard <previous commit>
careful though
it looks like you did the right thing
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 10:52
ah ok
then I'll leave it at that
so why isn't it CLS-Compliant?
Steven Liekens
@StevenLiekens
Sep 13 2015 10:54
because GW2NET.V2.Accounts doesn't include SharedAssemblyInfo
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 10:54
ah
Steven Liekens
@StevenLiekens
Sep 13 2015 10:54
open it with a text editor and add this block
<Compile Include="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), SharedAssemblyInfo.cs))\SharedAssemblyInfo.cs">
  <Link>Properties\SharedAssemblyInfo.cs</Link>
</Compile>
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 10:54
post me the details. I'll do it later. Going to eat with family
Steven Liekens
@StevenLiekens
Sep 13 2015 10:55
I might change the path to simply $(SolutionDir)SharedAssemblyInfo.cs
yeah I think I'll do that
Steven Liekens
@StevenLiekens
Sep 13 2015 11:05
I'm adding this element
<Compile Include="$(SolutionDir)SharedAssemblyInfo.cs">
  <Link>Properties\SharedAssemblyInfo.cs</Link>
</Compile>
assemblyinfo.png
should we document that somewhere?
because you have to remember to add that snippet every time you create a new project
unless it's a unit test project
Steven Liekens
@StevenLiekens
Sep 13 2015 11:15
PS
the merge commit that you wanted to undo was actually the result of a pull
I'm guessing Visual Studio did that
but there's a way to avoid that
if your local master branch has diverged
git fetch
git rebase origin/master
and then it replays your commits on top of origin/master
without creating a merge commit
Steven Liekens
@StevenLiekens
Sep 13 2015 11:26
who wants to play some guild wars?
shurne
@SamHurne
Sep 13 2015 11:27
You're on EU right?
Steven Liekens
@StevenLiekens
Sep 13 2015 11:29
yep
shurne
@SamHurne
Sep 13 2015 11:33
I'm on NA. I'll have to make an EU F2P account sometime... been thinking of doing that anyways.
Steven Liekens
@StevenLiekens
Sep 13 2015 11:34
you can guest on other servers
shurne
@SamHurne
Sep 13 2015 11:35
Ah! Can you guest from NA to EU? I wasn't aware you could do that
Hmm just checked, it says you can only guest on servers in the same region
Steven Liekens
@StevenLiekens
Sep 13 2015 11:36
I tried to make a screenshot and it crashed my computer
oh
what's your home world?
shurne
@SamHurne
Sep 13 2015 11:37
Sea of Sorrows
Steven Liekens
@StevenLiekens
Sep 13 2015 11:38
I didn't know that
does that apply to megaservers?
shurne
@SamHurne
Sep 13 2015 11:39
Yea I think so
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 12:06
ok
I don't know why appveyor does not build
so i readded it
and now it builds without problems
I'll delete the old one and Update the readme
Steven Liekens
@StevenLiekens
Sep 13 2015 12:08
I'm gonna stop pushing to your account now
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 12:08
k
Steven Liekens
@StevenLiekens
Sep 13 2015 12:08
so it's not a big deal if it only builds when you push changes
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 12:08
Id wouldn't even build when I pushed changes
Steven Liekens
@StevenLiekens
Sep 13 2015 12:08
I think that might actually be a feature
oh :/
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 12:13
ok
now everything works fine
had to fix some other links in the readme
most of the links were still pointing to gw2dotnet
Steven Liekens
@StevenLiekens
Sep 13 2015 12:15
can we turn off the missing comment warnings?
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 12:15
I'll look into it
Steven Liekens
@StevenLiekens
Sep 13 2015 12:15
I don't understand why that's enabled by default
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 12:17
another thing I thought about when I coded my skyrim tools: Since we are using JSON.NET, we should change our converters to make use of the JSON Converter it offers
Steven Liekens
@StevenLiekens
Sep 13 2015 12:18
don't we use that already?
what's it for?
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 12:20
we don't use it
you write the converter and put an attribute on the models and json.net converts automatically
I have to look into it some more but if I got it right, we can remove all fileds like private readonly IConverter<IResponse<AccountDataContract>, Account> responseConverter;
fix'd
and I think if we do some more work, we don't need the data contracts any more
Steven Liekens
@StevenLiekens
Sep 13 2015 12:23
so you'd get rid of AccountDataContract and deserialize directly to Account?
using a custom converter
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 12:23
If I got everything right: yes
Steven Liekens
@StevenLiekens
Sep 13 2015 12:24
that's how I had it at one point
but I couldn't do it without putting custom attributes on the model classes
if you can do it without polluting Account with json.net attributes then I'd say do it
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 12:25
see here
you just put a [JsonConverter(typeof(TType))] on the model class
Steven Liekens
@StevenLiekens
Sep 13 2015 12:26
yeah that's where I have a problem because now you have to make the Core library depend on json.net
because the model class lives in the Core assembly
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 12:27
I don't see a problem in that
why would it be a problem?
now let's see if the new webhook works
Steven Liekens
@StevenLiekens
Sep 13 2015 12:30
before I answer that
what if there are many json representations?
how does it know which converter to choose
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 12:32
afaik you can specify multiple converters for one class
Steven Liekens
@StevenLiekens
Sep 13 2015 12:33
nope
wait what, ayoung?
one minute
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 12:34
In think I read something about your case
you can't specify it more than once
AllowMultiple = false
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 12:35
you wouldn't
there was something about an array of converters
But I don't know where I read it
there's this
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 12:36
ah yes
soemthing in that direction
Steven Liekens
@StevenLiekens
Sep 13 2015 12:37
I assume the way it works is you add two converters for the same type to that collection
and then override CanConvert
so it knows which converter to choose
if we can somehow pass the source URL to CanConvert then I think it's a good idea
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 12:40
that should be possible
and now appveyor works
nice
Steven Liekens
@StevenLiekens
Sep 13 2015 12:40
so AccountConverter would return true only if the source URL is /v2/account
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 12:40
yep
Steven Liekens
@StevenLiekens
Sep 13 2015 12:41
no damn it
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 12:41
I'll test it on a seperate branch for the builds endpoint
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 12:41
that one is simple enough
Steven Liekens
@StevenLiekens
Sep 13 2015 12:41
I guess it's not that simple
oh and
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 12:42
I mean the builds endpoint is simple for a test
Steven Liekens
@StevenLiekens
Sep 13 2015 12:42
I thought of another issue
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 12:42
no reason to test it on, say... items
Steven Liekens
@StevenLiekens
Sep 13 2015 12:42
sometimes we use HTTP headers
look at BuildConverter
it uses the Date header
return new Build
{
    BuildId = value.BuildId,
    Timestamp = response.Date
};
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 12:44
it should be possible to pass that, too. But I'll have to check that
Steven Liekens
@StevenLiekens
Sep 13 2015 12:46
I don't think it is :worried:
anyway I kinda like the separation that we get from using data contract classes
Steven Liekens
@StevenLiekens
Sep 13 2015 12:56
let me know if you find a way to do it without adding newtonsoft attributes to the models
I'm ready to tell you why it would be a problem
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 12:59
pray tell
Steven Liekens
@StevenLiekens
Sep 13 2015 13:04
can't tag the model class with a converter attribute unless the converter lives in the same assembly
// Account and AccountConverter have to be in the same project
[JsonConverter(typeof(AccountConverter))]
public class Account { }
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 13:04
ah, damn
Steven Liekens
@StevenLiekens
Sep 13 2015 13:05
you might decide to make it so, but then there's no point in keeping the V2.Account project
plus the other thing about not being able to specify multiple converters using attributes
but I do believe there is another way we can pull this off
one minute
Steven Liekens
@StevenLiekens
Sep 13 2015 13:11
// in V1.Builds 
public Build GetBuild()
{
    var response = @"{ "build_id" : 52947 }";
    return JsonConvert.DeserializeObject<Build>(response, new BuildConverterV1());
}

// in V2.Builds
public Build GetBuild()
{
    var response = @"{ "id" : 52947 }";
    return JsonConvert.DeserializeObject<Build>(response, new BuildConverterV2());
}
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 13:11
that should work, with that we can also specify more than one converter
Steven Liekens
@StevenLiekens
Sep 13 2015 13:12
:)
and still have access to response headers
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 13:13
yup
Steven Liekens
@StevenLiekens
Sep 13 2015 13:14
btw
uhm
what about error reporting
like when they add a field
Steven Liekens
@StevenLiekens
Sep 13 2015 13:16
if you use the default converter then you can configure it to throw an exception
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 13:16
see link
Steven Liekens
@StevenLiekens
Sep 13 2015 13:16
but for custom converters you have to implement your own error handling
sometimes I like to run all of the code with the MissingMemberHandling.Errorsetting
we should probably make that the default setting for unit tests
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 13:22
yup
public Build GetBuild()
{
    var response = @"{ "id" : 52947 }";
    return JsonConvert.DeserializeObject<Build>(response, new BuildConverterV2())
    {
        MissingMemberHandling = MissingMemberHandling.Error
    };
}
that should work
ofc we need to catch the error
Steven Liekens
@StevenLiekens
Sep 13 2015 13:24
the thing is that you need to write BuildConverterV2 in a way that checks for missing members
and I don't know how to do that reliably
suppose that they change the json for /v2/build
{
    "id" : 52947,
    "previousId" : 52902
}
maybe that's a weird example
but that should crash our code because our Build class doesn't have a PreviousId property
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 13:29
I think wrapping it in try/catch and catching the JsonSerializationExceptionshould work
I'll test it later today
I'll have to do some math for my analysis exam on the 15th
Steven Liekens
@StevenLiekens
Sep 13 2015 13:30
no that's the thing
your custom converter is responsible for throwing the JsonSerializationException
pfft
if (fieldName != "id" && settings.MissingMemberHandling = MissingMemberHandling.Error)
{
    throw new JsonSerializationException("Unexpected field: " + fieldName);
}

// Unexpected field: previousId
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 13:32
hehe
Steven Liekens
@StevenLiekens
Sep 13 2015 13:33
that's all hardcoded
it needs to be not hardcoded to be reliable
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 13:35
there is a way
we could save the Json.Schema and then validate it
This message was deleted
that is obsolete
but there is a new project from him
see here
Steven Liekens
@StevenLiekens
Sep 13 2015 13:39
but there are no official schemas
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 13:39
well getting them shouldn't be that hard
a) we can ask b) we can post a schema on Github as PR and let them "sign" those
Steven Liekens
@StevenLiekens
Sep 13 2015 13:56
okay but still
the schema that we have is only useful for as long as they don't change it
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 13:57
if they change it, we would have to change our code too
so that is not a problem
Steven Liekens
@StevenLiekens
Sep 13 2015 13:58
how will we detect that our schema is out of date?
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 13:59
if the validation fails, we can assume that the schema is out of date
else, the validation would not fail
or
we propose another endpoint for the api, which provides the schema for us
like /v2/schema
for all schemas
and v2/schema/"schematype"
Steven Liekens
@StevenLiekens
Sep 13 2015 14:05
btw can we please use a DI framework?
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 14:07
after integrating the missing endpoints that would be the next on my toDo list
Steven Liekens
@StevenLiekens
Sep 13 2015 14:08
so use a DI framework for the missing endpoints then? :smile:
Steven Liekens
@StevenLiekens
Sep 13 2015 14:35
can we make it so that there is no more GW.V1 or GW.V2 in the bootstrapper
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 14:36
I'd rather remove V1 from the library in the future
Steven Liekens
@StevenLiekens
Sep 13 2015 14:36
hear me out
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 14:36
the development is finished and there will be nothing to add
ok
tell
Steven Liekens
@StevenLiekens
Sep 13 2015 14:37
instead of GW.V1.Colorsor GW.V2.Colors
the bootstrapper would only have GW.Colors
and use the highest available version
based on what packages are installed
using MEF probably
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 14:42
seems reasonable
Steven Liekens
@StevenLiekens
Sep 13 2015 14:49
they changed /v2/account/characters to simply /v2/characters
should probably rename the project
Robert Logiewa
@Ruhrpottpatriot
Sep 13 2015 14:49
yup