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

Update copyright texts #1563

Merged
merged 1 commit into from Mar 12, 2019

Conversation

Projects
None yet
4 participants
@ergo720
Copy link
Contributor

ergo720 commented Mar 5, 2019

No description provided.

applications. This exception does not alter any other responsibilities of
the licensee under the GPL (meaning that such software must still obey
the GPL for the free ("open-sourced") code that has been integrated into
the said software).

This comment has been minimized.

@JayFoxRox

JayFoxRox Mar 8, 2019

Contributor

Couldn't this paragraph be relevant?

* o Allocate bandwidth in frames properly
* o Disable timers when nothing needs to be done, or remove timer usage
* all together.
* o BIOS work to boot from USB storage

This comment has been minimized.

@JayFoxRox

JayFoxRox Mar 8, 2019

Contributor

This TODO list seems unncessary for Cxbx-Reloaded / should be moved into issues.

What should be kept, is a commit revision of QEMU so people can find the history of this file + find out what needs to be updated in the future.

This comment has been minimized.

@ergo720

ergo720 Mar 9, 2019

Author Contributor
  1. Isochronous transfers are already implemented.
  2. Here, I think they are referring to section 3.4.2 (Bandwidth Allocation) of the OHCI standard. To my knowledge, indeed, the existing code doesn’t take into account any of this, it just transfers all packets until there are none left (or an error occurs). In practice, I’m not sure if it’s worth implementing since it seems to work fine even without.
  3. The only timer used in the code is the one that’s used to periodically run OHCI_FrameBoundaryWorker, which is responsible to process the packets, so it’s required and cannot be removed. It’s only ever disabled during a reset or suspend event or if an unrecoverable error interrupt is raised (aka a fatal error in OHCI terms), so this is already covered I’d say.
  4. This doesn’t apply to Cxbx-R.

/*
* PROJECT: ReactOS Kernel
* LICENSE: GPL - See COPYING in the top level directory

This comment has been minimized.

@JayFoxRox

JayFoxRox Mar 8, 2019

Contributor

Does not apply here probably?

This comment has been minimized.

@ergo720

ergo720 Mar 9, 2019

Author Contributor

It applies, see new commit

This comment has been minimized.

@JayFoxRox

JayFoxRox Mar 12, 2019

Contributor

Why is there a copy of the COPYING file here then, if this claims people should look in COPYING in the top level directory: Which one counts? This paragraph (which is not in the top level) or the file in the top level?

I think both are kind of bad, because the COPYING file chosen by Cxbx-R might have to be changed eventually, whereas this code can't be easily relicensed.

It seems much easier to remove the reference to the COPYING file in the top level directory OR create a COPYING_REACTOS / COPYING-GPLV2 (or better named) or something to reference something stable.

Also note that the GPLv2 text allows 2 variations of licensing: ~"GPLv2 or any later version" or ~"GPLv2 only". So usually the copyright header has to be explicit about this; I'm not sure how ReactOS handles this.

// 4) QEMU is a trademark of Fabrice Bellard.
//
// Fabrice Bellard and the QEMU team

This comment has been minimized.

@JayFoxRox

JayFoxRox Mar 8, 2019

Contributor

Why is this here? This seems like it should be part of docs - not some random source file.

Instead, if you took copyrighted code, the headers of that files should be here and it should be clear which copyright / license applies to which parts.

@@ -43,8 +43,7 @@ A million repetitions of "a"
/* #define LITTLE_ENDIAN * This should be #define'd already, if true. */
/* #define SHA1HANDSOFF * Copies data before messing with it. */

// Acknowledgment: see above
// https://github.com/clibs/sha1

This comment has been minimized.

@JayFoxRox

JayFoxRox Mar 8, 2019

Contributor

Why remove the link? This file is in the public-domain, so this old header is fine.
Having the source URL helps in finding the state of it + finding updates.

@PatrickvL

This comment has been minimized.

Copy link
Member

PatrickvL commented Mar 10, 2019

Is discussion finished on this, so it can be merged?

@JayFoxRox

This comment has been minimized.

Copy link
Contributor

JayFoxRox commented Mar 10, 2019

I never did a full review on this - I only skimmed over it and called out the biggest issues (turning them into examples, to fix a class of issues). I believe there's more stuff that's not fixed yet (in this PR, and possibly the codebase).

So even if the discussion is finished, it doesn't mean that the changes are correct or complete.
It should be the maintainers responsibility to review and make sure that copyright notices stay intact and remain correct (across the codebase).

@PatrickvL

This comment has been minimized.

Copy link
Member

PatrickvL commented Mar 10, 2019

Then maybe it's best to merge this and create an issue for the follow-up work

@ergo720 ergo720 force-pushed the ergo720:copyright_update branch from b9b05d7 to b3fd593 Mar 12, 2019

@ergo720

This comment has been minimized.

Copy link
Contributor Author

ergo720 commented Mar 12, 2019

I don't think I have anything else to commit to this, so it could be merged now

@RadWolfie

This comment has been minimized.

Copy link
Member

RadWolfie commented Mar 12, 2019

It would be great to have all 3 fixup commits merge into "Update copyright texts" commit through git rebase -i.

Comparing to develop branch, looks better now.

EDIT: Alternative method can be done by squash and merge option.

@ergo720 ergo720 force-pushed the ergo720:copyright_update branch from b3fd593 to 00cbd82 Mar 12, 2019

@RadWolfie RadWolfie merged commit b31f412 into Cxbx-Reloaded:develop Mar 12, 2019

1 check passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

// Copyright 2015 Citra Emulator Project
// Licensed under GPLv2 or any later version
// Refer to the license.txt file included.

This comment has been minimized.

@JayFoxRox

JayFoxRox Mar 12, 2019

Contributor

This file does not exist.

@@ -25,8 +25,32 @@
// *
// ******************************************************************

// Acknowledgment: XQEMU (GPLv2)
// Acknowledgment: QEMU hub device emulation as used in XQEMU (GPLv2)

This comment has been minimized.

@JayFoxRox

JayFoxRox Mar 12, 2019

Contributor

As much as I appreciate all those mentions of XQEMU, the upstream you should care about for this is QEMU.

@RadWolfie

This comment has been minimized.

Copy link
Member

RadWolfie commented Mar 12, 2019

create an issue for the follow-up work

I agree with @PatrickvL for simply make an issue ticket of affecting copyright files issue. Plus determine each file license with the right license. After all of us make the agreement with absolutely no confusion at all from the issue ticket's discussion. Finally, make one pull request to resolve them without the need to create new pull requests.

@ergo720 ergo720 deleted the ergo720:copyright_update branch Mar 13, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.