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

Differentiate Exceptions from License Submission #471

Closed
wants to merge 5 commits into from

Conversation

BassCoder2808
Copy link
Contributor

@@ -140,7 +140,10 @@ def generateLicenseXml(licenseOsi, licenseIdentifier, licenseName, listVersionAd
licenseOsi = "true"
else:
licenseOsi = "false"
license = ET.SubElement(root, "license", isOsiApproved=licenseOsi, licenseId=licenseIdentifier, listVersionAdded=listVersionAdded, name=licenseName)
if isException:
license = ET.SubElement(root, "exception", isOsiApproved=licenseOsi, licenseId=licenseIdentifier, listVersionAdded=listVersionAdded, name=licenseName)
Copy link

Choose a reason for hiding this comment

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

6% of developers fix this issue

E501: line too long (158 > 79 characters)

❗❗ 17 similar findings have been found in this PR

🔎 Expand here to view all instances of this finding
File Path Line Number
src/app/generateXml.py 146
src/app/generateXml.py 158
src/app/utils.py 72
src/app/utils.py 181
src/app/utils.py 183
src/app/utils.py 184
src/app/utils.py 187
src/app/utils.py 203
src/app/utils.py 205
src/app/utils.py 207

Showing 10 of 17 findings. Visit the Lift Web Console to see all.


ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sonatype-lift ignoreall

Copy link

Choose a reason for hiding this comment

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

The ignoreall command is active on this PR, all the existing Lift issues are ignored.

@@ -69,7 +69,7 @@ def checkPermission(user):
logger.error("Permission denied while accessing the github api.")
return False

def makePullRequest(username, token, branchName, updateUpstream, fileName, commitMessage, prTitle, prBody, xmlText, is_ns):
def makePullRequest(username, token, branchName, updateUpstream, fileName, commitMessage, prTitle, prBody, xmlText, plainText, isException, is_ns):
Copy link

Choose a reason for hiding this comment

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

9% of developers fix this issue

E302: expected 2 blank lines, found 1


ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.

if isException:
commit_url = "{0}repos/{1}/{2}/contents/src/exceptions/{3}".format(url, username, settings.NAMESPACE_REPO_NAME if is_ns else settings.LICENSE_REPO_NAME, fileName)
else:
commit_url = "{0}repos/{1}/{2}/contents/src/{3}".format(url, username, settings.NAMESPACE_REPO_NAME if is_ns else settings.LICENSE_REPO_NAME, fileName)
Copy link

Choose a reason for hiding this comment

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

3% of developers fix this issue

W291: trailing whitespace


ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.

if isException:
file_url = "{0}/contents/src/exceptions/{1}".format(TYPE_TO_URL_NAMESPACE[NORMAL] if is_ns else TYPE_TO_URL_LICENSE[NORMAL], fileName)
else:
file_url = "{0}/contents/src/{1}".format(TYPE_TO_URL_NAMESPACE[NORMAL] if is_ns else TYPE_TO_URL_LICENSE[NORMAL], fileName)
Copy link

Choose a reason for hiding this comment

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

3% of developers fix this issue

W291: trailing whitespace


ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.

body = {
"path":"src/"+fileName,
"message":commitMessage,
"content":fileContent,
"branch":branchName,
}
text_file_body = {
"path":"src/"+ textFileName,
Copy link

Choose a reason for hiding this comment

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

2% of developers fix this issue

E231: missing whitespace after ':'


ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.

text_file_body = {
"path":"src/"+ textFileName,
"message":commitMessage,
"content":textFileContent,
Copy link

Choose a reason for hiding this comment

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

2% of developers fix this issue

E231: missing whitespace after ':'


ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.

body = {
"path":"src/"+fileName,
"message":commitMessage,
"content":fileContent,
"branch":branchName,
}
text_file_body = {
"path":"src/"+ textFileName,
Copy link

Choose a reason for hiding this comment

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

41% of developers fix this issue

E225: missing whitespace around operator

❗❗ 4 similar findings have been found in this PR

🔎 Expand here to view all instances of this finding
File Path Line Number
src/app/utils.py 221
src/app/utils.py 221
src/app/utils.py 221
src/app/utils.py 221

Visit the Lift Web Console to find more details in your report.


ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.

@@ -154,10 +155,10 @@ def submitNewLicense(request):
# Check if the license text doesn't matches with the rejected as well as not yet approved licenses
if not matches:
xml = generateLicenseXml(licenseOsi, licenseIdentifier, licenseName,
listVersionAdded, licenseSourceUrls, licenseHeader, licenseNotes, licenseText)
listVersionAdded, licenseSourceUrls, licenseHeader, licenseNotes, licenseText, isException)
Copy link

Choose a reason for hiding this comment

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

17% of developers fix this issue

E128: continuation line under-indented for visual indent

❗❗ 2 similar findings have been found in this PR

🔎 Expand here to view all instances of this finding
File Path Line Number
src/app/views.py 161
src/app/views.py 982

Visit the Lift Web Console to find more details in your report.


ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.

@@ -152,5 +155,5 @@ def generateLicenseXml(licenseOsi, licenseIdentifier, licenseName, listVersionAd
points = insertOls(objList)
textElement = getTextElement(points)
license.append(textElement)
xmlString = ET.tostring(root, method='xml', encoding='unicode')
xmlString = ET.tostring(root, method='xml', encoding='unicode', pretty_print=True)
Copy link

Choose a reason for hiding this comment

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

E1123: Unexpected keyword argument 'pretty_print' in function call


ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.

@@ -0,0 +1,33 @@
# Generated by Django 3.2.18 on 2023-05-29 14:33

from django.db import migrations, models
Copy link

Choose a reason for hiding this comment

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

9% of developers fix this issue

E0401: Unable to import 'django.db'


ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.

@@ -152,5 +155,5 @@ def generateLicenseXml(licenseOsi, licenseIdentifier, licenseName, listVersionAd
points = insertOls(objList)
textElement = getTextElement(points)
license.append(textElement)
xmlString = ET.tostring(root, method='xml', encoding='unicode')
xmlString = ET.tostring(root, method='xml', encoding='unicode', pretty_print=True)
Copy link

Choose a reason for hiding this comment

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

12% of developers fix this issue

reportGeneralTypeIssues: No overloads for "tostring" match the provided arguments
  Argument types: (Element, Literal['xml'], Literal['unicode'], Literal[True])


ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.

@@ -0,0 +1,33 @@
# Generated by Django 3.2.18 on 2023-05-29 14:33

from django.db import migrations, models
Copy link

Choose a reason for hiding this comment

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

11% of developers fix this issue

reportMissingImports: Import "django.db" could not be resolved


ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.

@jlovejoy
Copy link
Member

@BassCoder2808 - a couple comments / suggestions:

  • the isOsiApproved tag is not needed/relevant for exceptions, neither is standardLicenseHeader so it would be good if the generated XML could omit those, if possible
  • in the submission form - will the submitter just see "Is Exception" and a text box to fill in? I think we could have that be a yes/no option?
  • we probably ought to also update the explanatory text to include "license or exception" on most of the fields. happy to help with that part

@BassCoder2808
Copy link
Contributor Author

BassCoder2808 commented May 31, 2023

Hi @jlovejoy for your comments, yes I will remove the isOsiApproved tag for the exception part and for the standardLicenseHeader it is automatically added when a user puts that so that shouldn't be a problem for the exception XML generation.

  • we probably ought to also update the explanatory text to include "license or exception" on most of the fields. happy to help with that part

This would be really helpful if you could help me with this @jlovejoy, till that time I will finish up with adding the tooltip then we can change the text inside the tooltip

src/app/forms.py Outdated
@@ -80,6 +85,7 @@ def __init__(self, *args, **kwargs):
shortIdentifier = forms.CharField(label='Short identifier', max_length=25)
sourceUrl = forms.CharField(label='Source / URL', required=False)
osiApproved = forms.CharField(label="OSI Status", widget=forms.Select(choices=OSI_CHOICES))
isException = forms.CharField(label="Is Exception", widget=forms.Select(choices=YES_NO_CHOICES))
Copy link

Choose a reason for hiding this comment

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

6% of developers fix this issue

E501: line too long (100 > 79 characters)


ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.

Copy link
Collaborator

@rtgdk rtgdk left a comment

Choose a reason for hiding this comment

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

Left some comments

if response.status_code == 200:
""" Creating Commit by updating the file """
data = json.loads(response.text)
file_sha = data["sha"]
body["sha"] = file_sha
if text_response.status_code == 200:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Some changes looks similar to #470 . Please look into working with multiple branches on git. Create PRs with the correct upstream branch (branch before this one)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have made a new PR #492 for this, like further moving forward I have to redo #470 because of the migrations issues which it has having, so I have incorporated all the changes which you had suggested in #470 in #492, so now there is an extra utils function as well. Let me know if anything else needs to be done in that

migrations.AddField(
model_name='licensenamespace',
name='text',
field=models.TextField(default=''),
Copy link
Collaborator

Choose a reason for hiding this comment

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

You will need to commit only changed migrations if this is fixed in the other PR

(False, 'No'),
)

class TooltipTextInput(forms.TextInput):
Copy link

Choose a reason for hiding this comment

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

8% of developers fix this issue

E302: expected 2 blank lines, found 1


ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.

src/app/forms.py Outdated
context['widget']['attrs']['title'] = self.tooltip
return context

class TooltipSelect(forms.Select):
Copy link

Choose a reason for hiding this comment

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

8% of developers fix this issue

E302: expected 2 blank lines, found 1


ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.

context['widget']['attrs']['data-toggle'] = 'tooltip'
context['widget']['attrs']['title'] = self.tooltip
return context

Copy link

Choose a reason for hiding this comment

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

12% of developers fix this issue

W293: blank line contains whitespace


ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.

src/app/forms.py Outdated

def render_options(self, choices, selected_choices):
option_html = super().render_options(choices, selected_choices)
option_html = option_html.replace('<option', f'<option data-toggle="tooltip" title="{self.tooltip}"')
Copy link

Choose a reason for hiding this comment

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

6% of developers fix this issue

E501: line too long (109 > 79 characters)

❗❗ 6 similar findings have been found in this PR

🔎 Expand here to view all instances of this finding
File Path Line Number
src/app/forms.py 104
src/app/forms.py 105
src/app/forms.py 106
src/app/forms.py 107
src/app/forms.py 108
src/app/forms.py 110

Visit the Lift Web Console to find more details in your report.


ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.

context['widget']['attrs']['title'] = self.tooltip
return context

class CustomSelectWidget(forms.Select):
Copy link

Choose a reason for hiding this comment

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

8% of developers fix this issue

E302: expected 2 blank lines, found 1


ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.


class CustomSelectWidget(forms.Select):
def __init__(self, *args, **kwargs):
self.tooltip = kwargs.pop('tooltip', '') # Get the tooltip text from widget attributes
Copy link

Choose a reason for hiding this comment

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

6% of developers fix this issue

E501: line too long (95 > 79 characters)


ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.


def get_context(self, name, value, attrs):
context = super().get_context(name, value, attrs)
context['widget'].update({'attrs': {'title': self.tooltip}}) # Add the title attribute to the widget
Copy link

Choose a reason for hiding this comment

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

6% of developers fix this issue

E501: line too long (109 > 79 characters)


ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.

@BassCoder2808
Copy link
Contributor Author

Hi @rtgdk , I have created a new pull request #492 because lately there has been some issues with the migration files, this was also experienced in #470.
I have made all the changes which you had suggest in #492, let me know if anything else needs to be done in that

@BassCoder2808 BassCoder2808 requested a review from rtgdk July 14, 2023 10:33
@rtgdk
Copy link
Collaborator

rtgdk commented Aug 24, 2023

@BassCoder2808 Can we close this PR too? #496 has all these changes right?

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.

3 participants