Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix compile error and copy right
  • Loading branch information
jcs090218 committed Sep 18, 2023
1 parent c189cf4 commit 69d7c23
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 4 deletions.
1 change: 0 additions & 1 deletion .github/workflows/test.yml
Expand Up @@ -20,7 +20,6 @@ jobs:
matrix:
os: [ubuntu-latest, macos-latest, windows-latest]
emacs-version:
- 26.3
- 27.2
- 28.2
- 29.1
Expand Down
12 changes: 9 additions & 3 deletions dart-mode.el
@@ -1,9 +1,15 @@
;;; dart-mode.el --- Major mode for editing Dart files -*- lexical-binding: t; -*-

;; Author: https://github.com/bradyt/dart-mode/issues
;; URL: https://github.com/bradyt/dart-mode
;; Copyright (C) 2015-2018 Natalie Weizenbaum
;; Copyright (C) 2018-2023 Brady Trainor
;; Copyright (C) 2023 Shen, Jen-Chieh

;; Author: Natalie Weizenbaum <nex342@gmail.com>
;; Maintainer: Brady Trainor
;; Jen-Chieh Shen <jcs090218@gmail.com>
;; URL: https://github.com/emacsorphanage/dart-mode
;; Version: 1.0.7
;; Package-Requires: ((emacs "24.3"))
;; Package-Requires: ((emacs "26.3"))
;; Keywords: languages

;; The author is Brady Trainor, but removed from keywords in attempt
Expand Down

22 comments on commit 69d7c23

@bradyt
Copy link
Collaborator

@bradyt bradyt commented on 69d7c23 Sep 25, 2023

Choose a reason for hiding this comment

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

Interesting change here. I guess being wrong and following conventions is more important than being vague and accurate.

Such is this miserable fucking existence called life.

@bradyt
Copy link
Collaborator

@bradyt bradyt commented on 69d7c23 Sep 25, 2023

Choose a reason for hiding this comment

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

@tarsius I handed this off to you expecting some reasonable behavior from emacsorphanage. Would you like to informally cosign the validity of the above changes?

@bradyt
Copy link
Collaborator

@bradyt bradyt commented on 69d7c23 Sep 25, 2023

Choose a reason for hiding this comment

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

@jcs090218 can you clarify if you've read the code and comments?

@jcs090218
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hi, @bradyt!

Which part are you concerning?

Copyright?

;; Copyright (C) 2015-2018  Natalie Weizenbaum
;; Copyright (C) 2018-2023  Brady Trainor
;; Copyright (C) 2023  Shen, Jen-Chieh

Or this?

;; Package-Requires: ((emacs "26.3"))

@jcs090218
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just let me know what makes you unhappy about it. We can simply revert it. 🤔

@bradyt
Copy link
Collaborator

@bradyt bradyt commented on 69d7c23 Sep 25, 2023

Choose a reason for hiding this comment

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

@jcs090218 can you clarify if you've read the code and comments?

I meant the entire file, dart-mode.el, and the code and comments contained there.

@tarsius

This comment was marked as off-topic.

@tarsius

This comment was marked as off-topic.

@jcs090218

This comment was marked as off-topic.

@tarsius
Copy link
Member

Choose a reason for hiding this comment

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

@bradyt It's unclear to me as well to which changes you object.

I have pointed out some things that IMO could have been done better, but at the same time I think that dropping support for Emacs 24 and 25 is a reasonable thing to do, and the updated copyright lines also seem reasonable to me. One can of course disagree. If you do, then please do so more explicitly.

@bradyt
Copy link
Collaborator

@bradyt bradyt commented on 69d7c23 Sep 25, 2023

Choose a reason for hiding this comment

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

Still waiting to hear who has read the code and comments in the file. That seems like a reasonable expectation.

@jcs090218

This comment was marked as off-topic.

@tarsius
Copy link
Member

Choose a reason for hiding this comment

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

Still waiting to hear who has read the code and comments in the file. That seems like a reasonable expectation.

I am unsure what you are asking.

If you are asking whether someone has reviewed the changes @jcs090218 has made before they hit the main branch, then the answer is that I have not done that. If that was indeed what you were getting at, then that probably means that you are dissatisfied by some of those changes, as well as by the fact that I did not review them.

But maybe you mean your question more literally. It seems logical to assume that @jcs090218 did read at least some of the code and comments already. However, since for now he has concentrated on CI and some metadata updates, he might not have studied everything in detail yet, and I would take no issue with that. But if you suspect this is the case and that is indeed what you are taking an issue with, then please say so.

I have not studied the code in detail either. That is reasonable because I don't intend to make any changes myself. I have scrolled through the file for a superficial assessment of the code quality. Seems generally reasonable. There seems to be a reasonable amount of comments, and they generally sound useful. There is a mixture of code that tries to stay below ~80 characters per line, but a smaller subset needlessly goes beyond that limit. That's something I would change, but it's nothing major. As I said, the code generally looks sound.

Maybe you are referring in particular to the comment in the header, before the commentary. I have read that. The first sentence could be understood as you saying that you are worried about robots scrapping your email address. It seems @jcs090218 interpreted it that way, after all he did omit your email address in the information he added. But maybe this goes a step further and you don't want to be named in a copyright line, that's another possible interpretation. If that is the case, I would like to learn why that is.

The following paragraph suggests that you have discarded a lot / most of the code originally written by @nex3. I have not studied the history beyond looking at the output of magit-log-current, so I cannot assess how much code, that was written before your time, remains. Maybe you object to not being listed as the author or one of the authors (as opposed to only being mentioned as a maintainer). If your objection is related to this, then please say so.

@bradyt
Copy link
Collaborator

@bradyt bradyt commented on 69d7c23 Sep 25, 2023

Choose a reason for hiding this comment

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

Is nex3 an author? Do they have copyright? And why does jcs090218 have copyright, but they're not listed as an author? Author of what? If it's meaningless, why was it written down?

@bradyt
Copy link
Collaborator

@bradyt bradyt commented on 69d7c23 Sep 25, 2023

Choose a reason for hiding this comment

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

Let's see if we can get authorship and copyright to just trickle its way up to Google or Microsoft.

@tarsius
Copy link
Member

Choose a reason for hiding this comment

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

Is nex3 an author?

Yes. I determined this like so:

$ git shortlog -ns
   109  Natalie Weizenbaum
    67  Brady Trainor
     8  Jen-Chieh Shen
     4  Harry Terkelsen
     3  Philipp
     3  Philipp Stephani
     2  bradyt
     1  Bob W. Hogg
     1  Daniel Davidson
     1  Erick Navarro
     1  Faried Nawaz
     1  Göktuğ Kayaalp
     1  Ivan Inozemtsev
     1  Jonas Bernoulli
     1  Mostafa Razavi
     1  Piotr Anielski
     1  Robert Ginda
     1  Tony Gentilcore
     1  dependabot[bot]
     1  dickmao
     1  İ. Göktuğ Kayaalp

Do they have copyright?

Authors are generally automatically assigned copyright, but it is possible that the copyright is owned by her employer instead. Let's go check.

In here last commit, caa7a19 (two commits after 1.0.3), we can find this in the header:

;; Author: Natalie Weizenbaum
[...]
;; Copyright (C) 2011 Google Inc.

So she does claim authorship and stated that Google Inc. owns the copyright.

  • In ad93bc1 you add yourself as an additional author and the only maintainer.
  • In c568118 you start claiming sole authorship.
  • In 3bac142 (1.0.7) you start saying that https://github.com/bradyt/dart-mode/issues is the "author" and add an "honorable mention to previous author, maintainer and contributors".
  • In 69d7c23 @jcs090218 changes the author back to Natalie, declares you and himself as the maintainers, and adds copyright lines for all three of you.

Looking at the diff for caa7a19..3bac142 it indeed looks like you did a complete rewrite. Virtually no code remained untouched.

I then looked at the relevant diffs of the commits in that range individually. It is hard to assess how much code was written from scratch and how much was transformed. I can see both kinds of changes, but when you transform existing code, it would be a lot of work to determine if you are modifying code you have previously written from scratch or code authored by someone else. But my impression is that you indeed have rewritten the whole thing, virtually from scratch.

If that is indeed your claim, then it would be helpful if you could explicitly state it. (Yes, the comment before the commentary already does so, but it leaves room for some doubt.) We could then remove here copyright line. (Though if @nex3 has the time, and is still reading this, I would like to hear her opinion as well.

Additionally you seem to object to @jcs090218 adding his own copyright this early in the game, i.e., before the scope of his contributions warrant that. To resolve this peacefully, I would suggest that he removes his copyright line until he has made more changes to dark-mode.el itself.

And why does jcs090218 have copyright

Everyone owns the copyright to their contributions (unless they transfer it to someone else). He has made contributions, so he owns the copyright for those.

The reason why he put his copyright statement in dark-mode.el, is that for Emacs packages, this is usually where one looks for such information concerning the project as a whole. This clearly is not ideal and I personally refrain from adding myself there until I have made considerable contributions where I add such a copyright statement for myself.

I can understand that you find this move upsetting, but please also try to understand it from his perspective. He committed himself to the thankless task of maintaining an abandoned package, and in all likelihood does intend to make changes to the code going forward.

And why does jcs090218 have copyright, but they're not listed as an author? Author of what? If it's meaningless, why was it written down?

By not listing himself as an author for the time being, he did communicate that his contributions are not considerable enough to be put on the same level as those by you. He did add himself as the maintainer, which is appropriate because the plan is that he will be the one dealing with bug reports, feature requests and support. To make that work easier he began by updating CI to his liking.

@jcs090218 you have done nothing wrong per se, by adding a copyright statement for yourself, but I agree that it was premature. We now know that such an action can upset people, who have written/maintained a package in the past. And there is nothing wrong with that either; it is understandable. So going forward, I would ask you to not add your copyright statement to libraries you have not yet contributed to in any substantial way.

@bradyt you were upset and I cannot fault you for that. It does however seem, that from that, you immediately jumped to assuming bad intent from Jen-Chieh, and failure on my part to something about that. It would have been more productive to explicitly state what the problem is, along the lines "No code by Nathalie remains and Jen-Chieh hasn't written any yet, so it does not seem appropriate to list them as copyright holders".

@jcs090218
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@jcs090218 you have done nothing wrong per se, by adding a copyright statement for yourself, but I agree that it was premature. We now know that such an action can upset people, who have written/maintained a package in the past. And there is nothing wrong with that either; it is understandable. So going forward, I would ask you to not add your copyright statement to libraries you have not yet contributed to in any substantial way.

No problem! I understand. :) I have removed myself from copyright in 9b069e4.

@nex3
Copy link
Collaborator

@nex3 nex3 commented on 69d7c23 Sep 26, 2023

Choose a reason for hiding this comment

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

I don't have a strong opinion about whether I'm still listed as an author or not. It's not really up to me whether Google claims copyright on this or not, but my informal recommendation is that it's generally best to play it safe and include them in the copyright line given that the code is modified from their base (even if it's largely unrecognizable, which I don't have an opinion on either).

@tarsius Thank you for putting so much effort into calming down this thread 💙

@tarsius
Copy link
Member

@tarsius tarsius commented on 69d7c23 Oct 2, 2023

Choose a reason for hiding this comment

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

I've made an attempt at clarifying the situation in b0fcc74. This is the best I can do within a reasonable time, with the information that has been given to me. (I've waited a bit before making this change, in the hope I would receive more information.)

@tarsius
Copy link
Member

@tarsius tarsius commented on 69d7c23 Oct 2, 2023

Choose a reason for hiding this comment

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

I am glad we could resolve this.

I'll just copy @bradyt response attached to b0fcc74, so the whole conversation can be found here:

Fantastic, I appreciate the improvement in accuracy applying a minimum of requiring my efforts to review work today, in 2023.

Thank you for time spent reviewing the history, and the call to skill required.

@bradyt
Copy link
Collaborator

@bradyt bradyt commented on 69d7c23 Oct 2, 2023

Choose a reason for hiding this comment

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

I appreciate that, I did not look forward to having to research events to provide a formal reply to the commit and thread above.

If it helps, I'll just provide my impression of events, in case the matter comes up again, and someone would like a previous maintainer's vague and imprecise recollection of events.

Disclaimer: I may not name things right, or have perfect recall of events, or their order, etc.

The project I encountered, I think nex3 provided the original implementation using cc-mode framework. So this provides syntax highlighting, indentation, and possibly other Emacs features. We might call it the dart-cc-mode code. There was also a collection of code added later to interact with a Dart process, maybe it provided things like code formatting, I don't recall now. We might refer to it as the dart-server code. My vague recollection is, someone else provided the initial contribution to the dart-server code. But I'm really not sure without checking. And a number of authors contributed commits to this, as you see in the list you generated above.

The dart-cc-mode code had some quirks, like in syntax highlighting of strings with single quotes, or some oddities in indentation that might not have been fixable simply using some of the cc-mode framework options. If I recall correctly, I think it was loosely based on java-mode, which uses cc-mode framework.

I think I submitted a pull request, and nex3 contacted me in private asking if I was willing to maintain the project. I started maintaining the project, and I vaguely recall trying to figure out if a reimplementation of dart-cc-mode was necessary, and if I found it tractable. The path of least resistance seemed to be to reimplement the syntax highlighting and indentation. I will say, I recall nex3 making a small comment somewhere, about introducing regressions, and it wasn't till much later that I had noticed any regressions. While I tried to keep an eye on the Dart spec as a loose reference, the only Dart code I was checking dart-mode against, was basic scripting or Flutter code. Later, I had brief encounters with more exotic Dart code, I think that may have been Dart code I found in the core, or in any basic AngularDart. I didn't keep notes, maybe as I browsed core, I may have added something to unit tests, but not always. But I think you'll find issues relative to what's allowable from the formal spec of Dart. One good example might be variable declarations broken up across multiple lines, if I recall correctly.

As I reimplented dart-mode, I think I wrote in commits whenever I was taking inspiration from other work. Easier to remember, is that I planned to study the go-mode's author's implementation of an indentation function, as it seemed an appropriate approach for dart-mode. I never made sufficient time to understand go-mode's, so dart-mode's current indentation function still lacks niceties like improving indentation after "continued statements", if I recall the name.

For syntax highlighting of strings, I surveyed many different Emacs major-modes.

I found font-lock-studio indispensable for figuring out syntax highlighting of functions, variable declarations, and anonymous functions.

And here is a vague recall of a rewrite, and being sole author. At some point, after I had reimplemented syntax highlighting, and indentation, and path of least resistance seemed to be to remove the cc-mode implementation, I think I realized that it no longer used anyone else's code. There was still the dart-server code, which I determined did not need to be in the same project, since I didn't even use that, and there were competing tools like eglot, lsp-mode, and reformatter. And as those latter tools allude to, (authoring and) maintaining a tool like dart-server seemed to have diminishing returns. I was also trying to introduce CI (if I'm naming that right), and CI for dart-server code introduced a complexity for maintenance. So I think I used git to split that code out faithfully, maintaining the history of authorship. At that point, I realized all that was left was code I had written from scratch, ignoring wherever I took inspiration or study of other projects. I think you'll find longer commit messages when I felt it best to write how I arrived at a solution. If I was going to use go-mode's indentation function to figure out an improvement for dart-mode's, I expect I would make a note of it at least in the commit message, if not also in the codebase.

I guess at that point, I added the note you all found, that includes, "all the mistakes below are my own".

Given that the thread has significantly improved (from my perspective) with tarsius's work, I'm only mildly curious now, but still a little curious nevertheless, as the issue has been raised. What is the precedent for copyright in a rewrite? I know nex3 mentioned something about, "modified from a base", but I wasn't sure the precise meaning.

Finally, as a disclaimer for rambling, "if I had more time, this note would be shorter."

@tarsius
Copy link
Member

@tarsius tarsius commented on 69d7c23 Oct 5, 2023

Choose a reason for hiding this comment

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

Thanks a lot for that summary!

Finally, as a disclaimer for rambling, "if I had more time, this note would be shorter."

I also sometimes add that disclaimer. ;)

Given that the thread has significantly improved (from my perspective) with tarsius's work, I'm only mildly curious now, but still a little curious nevertheless, as the issue has been raised. What is the precedent for copyright in a rewrite? I know nex3 mentioned something about, "modified from a base", but I wasn't sure the precise meaning.

As with all matters that involves the law, you'll never know, unless a court gets involved. Which we have no reason to believe, will happen here.

Given your summary and my own observations, I think it would make sense to move Google's copyright line, more or less like this:

--- a/dart-mode.el
+++ b/dart-mode.el
@@ -1,6 +1,5 @@
 ;;; dart-mode.el --- Major mode for editing Dart files -*- lexical-binding: t; -*-
 
-;; Copyright (C) 2011-2018  Google Inc.
 ;; Copyright (C) 2018-2023  Brady Trainor
 
 ;; Author: Brady Trainor
@@ -14,13 +13,12 @@
 
 ;; To a not fully determined extend, this implementation derives from an
 ;; earlier implementation by Natalie Weizenbaum, for which the copyright
-;; holder is Google.
+;; is:
+;;
+;; Copyright (C) 2011-2018  Google Inc.
 
-;; In the discussion attached to [1], Brady Trainor implicitly repeats
-;; his claim, that no code from that earlier implementation remains.
-;; This seems plausible, but, as mentioned in that discussion, would have
-;; to be studied in more detail, to be absolutely certain, which is why I
-;; am restoring Google's copyright notice.
+;; In the discussion attached to [1], Brady Trainor states that no code
+;; from that earlier implementation remains.

Please sign in to comment.