Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: standardize license headers #1061

Merged
merged 33 commits into from
Nov 3, 2022
Merged

Conversation

kevgo
Copy link
Contributor

@kevgo kevgo commented Oct 7, 2022

Add license headers clarifying the copyright of source code files.

Related Issue or Design Document

https://github.com/ory-corp/general/issues/602

Checklist

  • I have read the contributing guidelines and signed the CLA.
  • I have referenced an issue containing the design document if my change introduces a new feature.
  • I have read the security policy.
  • I confirm that this pull request does not address a security vulnerability.
    If this pull request addresses a security vulnerability,
    I confirm that I got approval (please contact [email protected]) from the maintainers to push the changes.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added the necessary documentation within the code base (if appropriate).

@kevgo kevgo requested review from zepatrik and hperl as code owners October 7, 2022 22:45
@kevgo kevgo marked this pull request as draft October 7, 2022 23:14
@kevgo
Copy link
Contributor Author

kevgo commented Oct 7, 2022

Blocking on shipping ory/cli#239 as a new Ory CLI version.

Copy link
Collaborator

@hperl hperl left a comment

Choose a reason for hiding this comment

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

Thanks! I really like having unified license headers across our projects! I have just left some comments & questions 😃

Makefile Outdated
@@ -160,3 +161,7 @@ post-release: tools/yq
.PHONY: generate
generate: tools/stringer
go generate ./...

.bin/ory: Makefile
curl https://raw.githubusercontent.com/ory/meta/master/install.sh | bash -s -- -b .bin ory v0.1.43
Copy link
Collaborator

Choose a reason for hiding this comment

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

How do we update the version here? Manually? All other dependencies are tracked in Homebrew or in .bin/gobin/go.mod. Can we put it there?

Copy link
Member

@zepatrik zepatrik Oct 10, 2022

Choose a reason for hiding this comment

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

We already have the Ory CLI checked in through homebrew, this can just be skipped all together 😉
Just use ory as the command in the makefile, it is already set in the PATH correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback! This brings up a larger conversation (than fits into this PR), so I have opened https://github.com/ory-corp/general/issues/754 to discuss the various approaches to install dependencies, the thinking behind them, and how we might simplify the current complexity around dependency management in all repos.

@hperl yes I was planning on bumping the version manually if needed, through a PR that verifies that everything still works. That's the most simple, straightforward, and robust solution in my experience, and anybody can do it. I created https://github.com/kevgo/mrt to automatically perform such updates for all Ory repos if needed.

@zepatrik how do I install the Ory CLI using Homebrew on Linux (Debian Bullseye)? I just ran make sdk and make tools/cli (which I guess will create a tools/ory command?) and both ran for 5 minutes each and then errored with:

Warning: No available formula with the name "keto/tools/[email protected]".
==> Searching for similarly named formulae...
Error: No similarly named formulae found.
==> Searching for a previously deleted formula (in the last month)...
Error: No previously deleted formula found.
make: *** [Makefile:37: tools/go-swagger] Error 1

Given that the ORY CLI is a simple stand-alone Go binary with a custom binary name, why aren't we installing it through its cross-platform installer script? That takes only a few seconds.

Copy link
Member

Choose a reason for hiding this comment

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

Given that the ORY CLI is a simple stand-alone Go binary with a custom binary name, why aren't we installing it through its cross-platform installer script? That takes only a few seconds.

This will work for Ory CLI, but not for protoc 😞
I am not sure why it failed for you, but I agree that the brew variant sucks. I will write an install script for protoc instead that pulls the binary from github. I'll try to contribute it to https://github.com/protocolbuffers/protobuf so we don't have to maintain it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds great! Here is a super simple template in case you need one: https://github.com/git-town/git-town/blob/main/website/scripts/install_mdbook.

@@ -1,3 +1,5 @@
// Copyright © 2022 Ory Corp
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why don't we put the full declaration on each file as per Apache License?

   Copyright 2022 Ory Corp

   Licensed under the Apache License, Version 2.0 (the "License");
   you may not use this file except in compliance with the License.
   You may obtain a copy of the License at

       http://www.apache.org/licenses/LICENSE-2.0

   Unless required by applicable law or agreed to in writing, software
   distributed under the License is distributed on an "AS IS" BASIS,
   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
   See the License for the specific language governing permissions and
   limitations under the License.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hperl Thanks for your suggestion. This question is still open. Please find the discussion around the license headers in https://github.com/ory-corp/general/issues/602. I went with the shortest possible option primarily because it is short. The headers can be changed later as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hperl FYI based on your suggestion I am now including the license name, but abbreviated in a standard format so that it is machine readable.

Makefile Show resolved Hide resolved
@kevgo kevgo changed the title chore: add license headers docs: standardize license headers Nov 3, 2022
@kevgo kevgo marked this pull request as ready for review November 3, 2022 12:24
@kevgo kevgo merged commit 6c0e1ba into ory:master Nov 3, 2022
@kevgo kevgo deleted the kg-license-headers branch November 3, 2022 17:33
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.

5 participants