Skip to content

Conversation

@sungam3r
Copy link
Member

@sungam3r sungam3r commented Oct 29, 2020

  • add GitHub Actions
  • remove AppVeyor and JS tools
  • Bump deps
  • add .editorconfig
  • add API approval test
  • add badges
  • add package icon
  • add SourceLink
  • add Playground/GraphiQL launch setting in Harness
  • README tweaks

- add GitHub Actions
- remove AppVeyor and JS tools
- Bump deps
- add .editorconfig
- add badges
- add package icon
- add SourceLink
- add Playground/GraphiQL launch setting in Harness
- README tweaks
@github-actions github-actions bot added CI CI configuration issue or pull request code style Pull request that applies code style rules documentation test Pull request that adds new or changes existing tests labels Oct 29, 2020
@sungam3r
Copy link
Member Author

@Shane32 @joemcbride I will be grateful for the review. I was finally able to get on with updating this repository.

@Shane32
Copy link
Member

Shane32 commented Oct 29, 2020

I glanced over everything @sungam3r . I've never used this repo but looks like a lot of copy and paste of the work we've been doing in the other repos recently. 👍

@sungam3r sungam3r merged commit e450dcf into master Nov 1, 2020
@sungam3r sungam3r deleted the github-actions branch November 1, 2020 13:32
<ItemGroup>
<PackageReference Include="GraphQL" Version="3.0.0.2026" />
<PackageReference Include="GraphQL" Version="3.1.3" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's changes like these that make me wonder why separate repos are a benefit over mono repos. Since, even though 3.1.0 is binary compatible with 3.1.3, now users are REQUIRED to use 3.1.3 to gain the latest benefits from this package.

If we are going to have separate repos, each repo should target the oldest compatible version - in this case, probably 3.0.0.

If not, we might as well have a "mono repo".

cc: @joemcbride

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed with this, minimal version requirements, otherwise mono repo is preferred.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Targeting 3.0.0 vs 3.1.0 vs 3.1.3 vs 3.x.x is an old question. Each dependency does some work. It's not just about binary compatibility and targeting the smallest possible version. It's about getting the job done correctly. If initially I used the dependency version 3.0.0, and then version 3.0.1 came out with a bug fix, then I prefer to use it, increasing the dependency explicitly. If then version 3.1.0 is released, the new feature of which I do not use, then I most likely prefer not to change the version of the dependency. But if in the next release 3.1.1 some bug is fixed again (which most likely affects clients), then I again prefer to use the dependency of version 3.1.1. This is the logic.

I have already described the disadvantages of mono repo. They may seem more comfortable than several separate ones, but this impression is deceiving. Perhaps this is a matter of habit. I am aware that in some scenarios it is easier to merge everything into one repository. For example, as the .NET team did with runtime repo. In my practice, I have to deal with building a technological stack of libraries for the operation of dozens of microservices. This stack contains several dozen repositories, which are logically (but not physically) interconnected in a hierarchical structure. Each layer is ordered and serves its purpose. The so-called SDK is located on the upper layer. It's kind of Microsoft.AspNetCore.App but only for internal development. At its level, I can set the required versions of all downstream dependencies that will be consumed by microservices. Microservices only refer to this SDK (single nuget, metapackage), and not to its individual components. In the case of a mono repository, I simply would not be able to build such a system. And this is not touching upon the issues of administration, when you need to give rights to someone, and not to give to someone.

@sungam3r
Copy link
Member Author

sungam3r commented Nov 2, 2020

Since, even though 3.1.0 is binary compatible with 3.1.3, now users are REQUIRED to use 3.1.3 to gain the latest benefits from this package.

Exactly, required. Because we as developers know what bugs were fixed in version 3.1.3 and preferred to use this particular version.

And one bonus - the client can always refer to a lower version of the dependency. Yes, I didn't know that until relatively recently. Just try it. First you will see NU1605:

Error	NU1605	Detected package downgrade: GraphQL.SystemTextJson from 3.1.3 to 3.1.2. Reference the package directly from the project to select a different version. 
 GraphQL.Samples.Server -> GraphQL.Server.Transports.AspNetCore.SystemTextJson -> GraphQL.SystemTextJson (>= 3.1.3) 
 GraphQL.Samples.Server -> GraphQL.SystemTextJson (>= 3.1.2)	Samples.Server

(treatwarningsaserrors=true). Ignore this warning (<NoWarn>$(NoWarn);1591;NU1605</NoWarn>) and try again and voila:

Successfully uninstalled 'GraphQL.SystemTextJson 3.1.3' from Samples.Server
Successfully installed 'GraphQL.SystemTextJson 3.1.2' to Samples.Server
Committing restore...

So I don't see a problem in bumping the dependencies. For most consumers, this is more of a benefit than harm. And those who really, for some reason, need to use the old version of the dependency can always do it.

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

Labels

CI CI configuration issue or pull request code style Pull request that applies code style rules documentation test Pull request that adds new or changes existing tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants