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

Revert "Revert "Restore example history"" #386

Conversation

calebfoss
Copy link
Contributor

@davepagurek I realized I can open this PR to revert the revert. Is it possible to try the merge and rebase option with this?

@davepagurek davepagurek merged commit dc693ea into processing:main May 29, 2024
4 checks passed
@davepagurek
Copy link
Collaborator

I tried a rebase and merge, let me know if it worked!

@calebfoss
Copy link
Contributor Author

Alas it did not. Thanks for trying! I think someone with access to the main branch is going to need to rebase to fix this. I don't think I will be able to take care of it on my end.

@limzykenneth
Copy link
Member

The history is already in the trunk so it can't be readded, even locally, except if we do a force push that may cause other problems with the history. I've not really seen this done before in general so I don't really know how to effectively merge two unrelated repo.

@calebfoss
Copy link
Contributor Author

calebfoss commented Jun 11, 2024

Hi @limzykenneth! I want to be mindful of tone here. My goal is to set a tone that is respectful, compassionate toward your position, and firm on the importance of attribution.

I want to recount the order of events from my POV to help contextualize my position:

  1. Last fall I led a team to revamp the examples as part of the STF documentation project. @Qianqianye set up a separate repo for the project, and I used a sparse checkout from the p5js website repo to maintain the git history. We added examples from @Malayvasa's GSoC project, which had been submitted on the p5 editor rather than github, so I added attribution in the commit message.
  2. In the readme for the project, I included a note about maintaining the git history.
  3. This spring, I converted the JSDoc for the examples to MDX. At the time, I did not have any access to the new p5 website development repo. When corresponding with @stalgiag about this, I brought up maintaining the git history for the examples. I requested access to the repo so that I could open a PR to merge in the examples from the examples repo, but I did not get access at the time.
  4. @Qianqianye tagged me on an issue on the new website repo about example thumbnails missing alt text. This is when I saw that the repo was now public and that you had pushed the commit that copied over all the code from the examples repo without attribution.
  5. I reached out to you and @Qianqianye via email about the missing attribution on the examples. Y'all brought up plans for a separate post with credit to all the STF contributors (which is awesome, but I still think proper attribution in the codebase is important). They said they'd definitely like to keep the git history, and you said that you could possibly try merging the histories if I think that maintaining it is important.
  6. In my reply, I suggested that when we do merge the histories, we use the Github api to populate a list of contributors for each example on the corresponding web page, and if that were unpractical, I suggested we could manually add the contributors to the MDX files. I asked for your thoughts but did not hear back.
  7. I opened this PR in an attempt to fix the issue. @davepagurek helped out getting that ready and merged. While this did re-add all the lost history, which is now part of this repo, Github still shows your commit as the only history on the individual current files.

To be clear, I know you care about p5 contributors, and I certainly do not think you are trying to plagiarize. I think this was an honest mistake in the middle of a super complex project with a lot of moving parts, and I certainly relate! Sometimes mistakes on the main branch happen, and while a force push is not ideal, I would argue that on a community-driven open source project, removing attribution on years on contributions without permission from contributors is worse. While I would still strongly encourage you to restore the history, I think the next best think would be for you to manually add all the contributors to each example in the MDX and then render that data on the website, as I suggested in the email I mentioned above.

(edited to include link to your commit)
(edited again to correct link to the note in the examples project readme)

@limzykenneth
Copy link
Member

Sorry if I have to come across as a bit blunt and I know it is not your intention but I really don't appreciate repeated veiled accusation of intentional erasure of you and other example author's efforts here by me.

I have made it clear multiple times in the email and here that I do not know how to merge two repos of different histories together, and despite that I have given suggestions on how may be done. I don't think it is fair that merging two repo with different histories being a difficult thing to do is attributable to me wanting to erase someone else's contributions. I really find that accusation rather upsetting. By the time I have got around to your email reply in 6, you have already filed the previous PR to restore the history and if that had successfully restore the commit history, it would have superceded the need to manually add contributors to the files (if that is still required, we'd happily take PR or do it ourselves if needed).

Specifically to quote from my reply email

In addition, for examples you have authored, you can directly include attribution to it in the example itself such as this one: https://p5js.org/examples/simulate-snowflakes.html. I personally think this provides more visibility and value on the authorship of the example than having it nested deep in git history.

If you’d like to action any of the above, let us know or feel free to file a PR directly against the library repo as necessary

Force pushing the repo risk erasing the commit history of other contributors, if I can't ensure that won't happen, there is little point to add attribution to some contributors while erasing others.

@calebfoss
Copy link
Contributor Author

I am so sorry for upsetting you, @limzykenneth. I want to emphasize that I was being honest and sincere when I wrote this:

To be clear, I know you care about p5 contributors, and I certainly do not think you are trying to plagiarize. I think this was an honest mistake in the middle of a super complex project with a lot of moving parts, and I certainly relate!

I am sorry that my communication had the impact of an unintended accusation. My goal has always been to restore attribution, and I put a lot of time and effort into the PR attempting to (unfortunately unsuccessfully) remedy the issue, taking your suggestions into account.

By the time I have got around to your email reply in 6, you have already filed the previous PR to restore the history and if that had successfully restore the commit history, it would have superceded the need to manually add contributors to the files (if that is still required, we'd happily take PR or do it ourselves if needed).

That makes sense! In reference to the part of your email that you quoted, I would note that I am advocating for attribution for everyone who worked on these examples, not just me, so I'm glad to hear you are on board with adding the contributors in the MDX.

Force pushing the repo risk erasing the commit history of other contributors, if I can't ensure that won't happen, there is little point to add attribution to some contributors while erasing others.

I had no idea! That makes sense to be cautious of that risk. It sounds like I was speaking from a place of ignorance, and I apologize.

As long as you plan to restore attribution in the codebase in one way or another before the new site goes public, I am happy to trust you with that and drop out of the conversation.

I am very sorry for upsetting you. You do great work, and I appreciate what you do!

@Qianqianye
Copy link
Collaborator

Thank you all for your input. We will test different options to solve this and get back to @calebfoss this week.
Just to confirm, @calebfoss, will you be open to take the task on adding the author and reviewer info in the MDX file for each example if we decide to do so? I'm not sure if I have the best knowledge on who worked on each example, but I can support the process of adding the info.

@calebfoss
Copy link
Contributor Author

@Qianqianye thank you! I have the version of this repo locally that does have the history matched up with the example files, so accessing the information would be straightforward. I think transcribing the authors to the MDX for each example may be a bit time consuming (I'm estimating couple hours). Would you be open to compensating that work?

@calebfoss
Copy link
Contributor Author

No worries if not! I remembered that you can also see the correct history on this repo! To do that, open this commit, navigate to each of the code files, and open the Blame panel. For example, here is the blame for the snowflakes example.

I hope you are able to fix the history issue! If you do not, you can amend @limzykenneth's commit with attribution using the co-authored-by convention. Here's documentation on that. I didn't follow that syntax when crediting @Malayvasa, so I would love to fix that on my commit as well.

@Qianqianye
Copy link
Collaborator

Thanks @calebfoss. I followed up with you in an email. More soon!

@calebfoss
Copy link
Contributor Author

Got it! Since you are not anticipating adding the attribution until after the site goes public and especially since several of the contributors on the examples were not part of the STF project (and I assume will not be included in that post), I opened #412 to add a placeholder in the meantime with a link to the blame for each file.

@calebfoss calebfoss mentioned this pull request Jun 19, 2024
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.

4 participants