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

Open source #86

Merged
merged 8 commits into from
Jun 6, 2018
Merged

Open source #86

merged 8 commits into from
Jun 6, 2018

Conversation

dkstevekwak
Copy link
Contributor

No description provided.

@Jolg42
Copy link

Jolg42 commented Jun 6, 2018

This doesn't look like an appropriate license for demos 🤔

@macdonst
Copy link

macdonst commented Jun 6, 2018

@dkstevekwak the LICENSE file in the root of the PR is MIT and that looks good but the Adobe Confidential header seems wrong to me. This is sample code we want folks to use, I don't think we want to retain rights like that header suggests. I mean is says:

Dissemination of this information or reproduction of this material is strictly forbidden unless prior written permission is obtained from Adobe.

So that ain't right.

Is that what you meant @Jolg42?

@stevengill
Copy link

This PR looks good on first glance to me.

@Jolg42 the license is MIT. The source code header is more about adobe copyright rather than license. It is the standard one we use here at Adobe. It does feel a little heavy handed though.

@stevengill
Copy link

lol I see at @macdonst beat me to it. I'll check with Adobe lawyers about it. It feels too heavy for this case.

@macdonst
Copy link

macdonst commented Jun 6, 2018

@stevengill yeah, we should get some clarification and add it to the handbook. I really feel sample/demo code should be MIT while everything else we open source should be Apache-2.0.

@Jolg42
Copy link

Jolg42 commented Jun 6, 2018

+1 @macdonst and the new repo https://github.com/Adobe-CEP/Getting-Started-guides is Apache 2.0 which is a lot better.

@stevengill
Copy link

Alright, chatted with @macdonst and a few others and we agree. It is to heavy handed and not good for open source. The apache 2.0 header is great and simple. Most new OS requests at Adobe go Apache license. So we haven't really run into this problem much. But now that we are suggesting MIT for sample code, this header needs to be updated.

I've updated our internal handbook.

The new header should be

ADOBE CONFIDENTIAL

Copyright [first year code created] Adobe

All Rights Reserved.
NOTICE: Adobe permits you to use, modify, and distribute this file in
accordance with the terms of the Adobe license agreement accompanying
it. If you have received this file from a source other than Adobe,
then your use, modification, or distribution of it requires the prior
written permission of Adobe. 

@Jolg42
Copy link

Jolg42 commented Jun 6, 2018

Sounds good! 👍

@dkstevekwak
Copy link
Contributor Author

@Jolg42 & @macdonst Thanks for your feedback and thanks @stevengill for providing a new template. I will replace the license headers tomorrow.

@@ -88,7 +89,7 @@ $._AA_={
currentChild = currentItem.children[j+1];
if (currentChild){
return $._AA_.searchBinForProjItemByName(0, currentChild, nameToFind);
} else
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bbb999 wanted to double check this change is fine. My editor was throwing an error since this is an extra else

@bbb999 bbb999 merged commit 8ff00ba into Adobe-CEP:master Jun 6, 2018
@bbb999
Copy link
Contributor

bbb999 commented Jun 6, 2018

Thanks!

@@ -48,7 +48,7 @@ function getDocumentInfo()
//
function setCTI(percent)
{
if (app.activeDocument && app.activeDocument.reflect.name = 'WaveDocument')
if (app.activeDocument && app.activeDocument.reflect.name === 'WaveDocument')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bbb999 wanted to confirm with you this change is ok. I think it should be === instead of =? please correct me if I'm wrong

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.

None yet

5 participants