-
-
Notifications
You must be signed in to change notification settings - Fork 346
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
Conversation
Blocking on shipping ory/cli#239 as a new Ory CLI version. |
There was a problem hiding this 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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Co-authored-by: Henning Perl <[email protected]>
# Conflicts: # Makefile
…_tuples_pb.js version_pb.js write_service_pb.js
# Conflicts: # Makefile # proto/ory/keto/opl/v1alpha1/syntax_service_pb.js
# Conflicts: # Makefile # package-lock.json
Add license headers clarifying the copyright of source code files.
Related Issue or Design Document
https://github.com/ory-corp/general/issues/602
Checklist
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.