Skip to content

Conversation

@sblom
Copy link
Member

@sblom sblom commented Oct 2, 2020

I made myself a Dockerfile so that I could test my previous PR against Linux locally, and figure I should share it.

There's definitely zero urgency to this PR. As for the flurry of activity, I ended up taking a some days off to relax at home, and this seemed like a productive use of my energy.

By the way, can I have write (not maintainer or admin) on the linked-data-dotnet/json-ld.net repo so that I can do pull requests out of "real" branches instead of my private fork?

@codecov
Copy link

codecov bot commented Oct 2, 2020

Codecov Report

Merging #81 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #81   +/-   ##
=======================================
  Coverage   68.74%   68.74%           
=======================================
  Files          23       23           
  Lines        4067     4067           
  Branches     1033     1033           
=======================================
  Hits         2796     2796           
  Misses       1127     1127           
  Partials      144      144           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ff57ebe...7350960. Read the comment docs.

Copy link
Member

@goofballLogic goofballLogic left a comment

Choose a reason for hiding this comment

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

This seems legit to me, but if we put it at the root, it is lending some special significance to running .net core 2.1 on ubuntu 18. What might be helpful is to have a build folder with various build assets in it, but it seems strange to pick and choose particular dockerfiles for that purpose. Not sure what to advise here. @asbjornu ?

@goofballLogic
Copy link
Member

goofballLogic commented Oct 3, 2020

Regarding

By the way, can I have write (not maintainer or admin) on the linked-data-dotnet/json-ld.net repo so that I can do pull requests out of "real" branches instead of my private fork?

Is there a particular benefit to having branches internal to the project? It only seems to make sense to me for long lived branches which will have multiple contributors (and so no natural owner). Is it easier if people create these branches inside the repo for some reason?

I must admit I have done it recently without thinking, but perhaps I should be using the forking workflow myself.

Copy link
Member

@asbjornu asbjornu left a comment

Choose a reason for hiding this comment

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

I've added @sblom to @linked-data-dotnet/contributors, so write access should be available. There is, however, little to gain from having "local" access besides secrets being available in builds (which are unavailable for forked pull requests for security).

Regarding this PR, I would love to see a test doing docker build and docker run of the image so we ensure that it doesn't regress over time.

@sblom
Copy link
Member Author

sblom commented Oct 4, 2020

Regarding this PR, I would love to see a test doing docker build and docker run of the image so we ensure that it doesn't regress over time.

That's a really good idea. I'll work on this.

Will docker run work from inside the docker container that's hosting the GitHub action? I suppose it will, but I admit that's a new trick for me.

@sblom sblom force-pushed the docker branch 3 times, most recently from ffc3348 to d2d05b5 Compare October 4, 2020 04:32
@sblom
Copy link
Member Author

sblom commented Oct 4, 2020

This seems legit to me, but if we put it at the root, it is lending some special significance to running .net core 2.1 on ubuntu 18.

It's a good point. The thing that keeps me fairly calm about having a single somewhat arbitrary standard docker build is that we're really more of a portable nupkg project than a Docker Containers kind of project. :)

For what it's worth, that choice of base container image wasn't entirely arbitrary. It's the oldest LTS .NET Core SDK in the Microsoft Container Registry running on the same LTS Ubuntu distro that GitHub Actions runs on.

With those parameters, if a developer runs dotnet test before pushing and has docker installed:

  1. We have confidence that we work on the most down-level runtime we can conveniently share among our entire contributor population, and
  2. PRs have a pretty good chance of passing the Test phase of our PR checks.

@sblom
Copy link
Member Author

sblom commented Oct 4, 2020

Will docker run work from inside the docker container that's hosting the GitHub action? I suppose it will, but I admit that's a new trick for me.

Totally worked. It's container inception, man.

@asbjornu asbjornu added this to the 1.1 milestone Oct 5, 2020
@sblom sblom force-pushed the docker branch 2 times, most recently from 45e262b to 4d49caa Compare October 5, 2020 23:22
@asbjornu asbjornu merged commit 5149724 into linked-data-dotnet:master Oct 8, 2020
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.

3 participants