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.

Sign up for GitHub

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

Jump to bottom

GitHub: Update copyright span #16085

Closed
wants to merge 1 commit into from
Closed

GitHub: Update copyright span #16085

wants to merge 1 commit into from

Conversation

Lamparter
Copy link
Contributor

@Lamparter Lamparter commented Aug 26, 2024 •
edited
Loading

Resolved / Related Issues

@Lamparter
Copy link
Contributor Author

@0x5bfa how did you edit the licence header in the resource script when you did in #14933 46716e9?

@yaira2 yaira2 changed the title Legal: Update copyright span GitHub: Update copyright span Aug 26, 2024
@0x5bfa

This comment was marked as outdated.

Sign in to view
@yaira2
Copy link
Member

yaira2 commented Aug 26, 2024

Whatever is easier to maintain

@0x5bfa

This comment was marked as outdated.

Sign in to view
@Lamparter
Copy link
Contributor Author

I was thinking of getting rid of copyright year.
This work is hard for the reviewer to review.
And once we did so, we don't have to touch here every year.

@0x5bfa do you want me to just make it 2018-present?

@0x5bfa

This comment was marked as outdated.

Sign in to view
@Lamparter
Copy link
Contributor Author

Lamparter commented Aug 27, 2024 •
edited
Loading

@0x5bfa 'See the LICENSE.' is grammatically incorrect. I recommend 'See the license file', 'See the license file in the root directory' or even just remove the 'See the license' sentence altogether

@0x5bfa

This comment was marked as outdated.

Sign in to view
@Lamparter
Copy link
Contributor Author

@0x5bfa @yaira2 I cannot resolve the merge conflicts, the editor just says Access denied: only those with write access to this repository can resolve these conflicts.

@0x5bfa

This comment was marked as outdated.

Sign in to view
@Lamparter Lamparter closed this Sep 4, 2024
@Lamparter
Copy link
Contributor Author

Oops, GitHub automatically closed since I wiped the PR clean of commit history. @0x5bfa I'm still working on this though.

@Lamparter Lamparter reopened this Sep 4, 2024
Lamparter added a commit to Lamparter/Files that referenced this pull request Sep 4, 2024
@Lamparter
📄 Update licence reference
6ef5cdf 
(again)
Resolves files-community#16085 (comment)
@Lamparter Lamparter marked this pull request as ready for review September 4, 2024 21:07
@Lamparter
Copy link
Contributor Author

@yaira2 do you not have a CODEOWNERS file?

0x5bfa
0x5bfa reviewed Sep 5, 2024
View reviewed changes
.gitignore Outdated Show resolved Hide resolved
.github/scripts/Configure-AppxManifest.ps1 Outdated Show resolved Hide resolved
src/Files.Core.SourceGenerator/Extensions/StringBuilderExtensions.cs Outdated Show resolved Hide resolved
@0x5bfa

This comment was marked as outdated.

Sign in to view
@Lamparter Lamparter requested a review from 0x5bfa September 5, 2024 06:39
0x5bfa
0x5bfa approved these changes Sep 5, 2024
View reviewed changes
Copy link
Member

@0x5bfa 0x5bfa left a comment

Choose a reason for hiding this comment

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

LGTM

src/Files.App.Server/Files.App.Server.csproj
@@ -1,4 +1,5 @@
<Project Sdk="Microsoft.NET.Sdk">
<!-- Copyright (c) 2018-2024 Files Community. Licensed under the MIT License. -->
Copy link
Member

Choose a reason for hiding this comment

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

For other reviewers: this is the reason why addition and deletion aren't the same count (newly the header added)

@0x5bfa 0x5bfa added the ready to merge Pull requests that are approved and ready to merge label Sep 5, 2024
@yaira2 yaira2 added needs - code review and removed ready to merge Pull requests that are approved and ready to merge labels Sep 5, 2024
@Lamparter Lamparter force-pushed the copyright branch 2 times, most recently from e53688e to 44f3295 Compare September 15, 2024 15:50
@Lamparter
Copy link
Contributor Author

I think I should close this - I initially opened this PR because I was worried about the law but I have come to realise that I'm really stupid this PR is absolutely unnecessary.

@Lamparter Lamparter closed this Sep 19, 2024
@Lamparter Lamparter deleted the copyright branch September 19, 2024 17:28
@0x5bfa
Copy link
Member

0x5bfa commented Sep 19, 2024

Probably someone or I do this when it becomes 2025.
I assume there aren't motivation to take an immediate action for this.

@Lamparter
Copy link
Contributor Author

Lamparter commented Sep 19, 2024 •
edited
Loading

Yeah.

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

@0x5bfa 0x5bfa 0x5bfa approved these changes

Assignees
No one assigned
Labels
needs - code review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Lamparter @0x5bfa @yaira2