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

Intended to solve #494 #495

Merged
merged 4 commits into from
Jul 28, 2023
Merged

Intended to solve #494 #495

merged 4 commits into from
Jul 28, 2023

Conversation

BassCoder2808
Copy link
Contributor

@vargenau
Copy link
Contributor

Hi @BassCoder2808,
Did you test your changes?
In the third case, I suppose you should have:

warnings = str(warnings)

Minor update on the variable name
@BassCoder2808
Copy link
Contributor Author

Hi @vargenau yes I just saw that the 3rd case had the warnings string, now I have made that change

@vargenau
Copy link
Contributor

Hi @vargenau yes I just saw that the 3rd case had the warnings string, now I have made that change

Thank you @BassCoder2808

Copy link
Member

@goneall goneall left a comment

Choose a reason for hiding this comment

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

Thanks @BassCoder2808 - just a minor suggested change to format the raw HTML a bit better

src/app/core.py Outdated
@@ -221,7 +221,8 @@ def ntia_check_helper(request):
""" If any warnings are returned """
if (request.is_ajax()):
ajaxdict["type"] = "warning"
ajaxdict["data"] = "The following warning(s) were raised:\n" + str(retval)
warnings = str(retval)
ajaxdict["data"] = "The following warning(s) were raised:\n" + warnings.replace('\n', '<br />')
Copy link
Member

Choose a reason for hiding this comment

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

Just a nit - it would be nice to keep a linefeed in case anyone looks at the raw HTML.

Copy link
Contributor

Choose a reason for hiding this comment

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

We could also have

"The following warning(s) were raised:<br />\n"

instead of:

"The following warning(s) were raised:\n"

so that the first warning is not on the same line.

src/app/core.py Outdated
@@ -221,7 +221,8 @@ def ntia_check_helper(request):
""" If any warnings are returned """
if (request.is_ajax()):
ajaxdict["type"] = "warning"
ajaxdict["data"] = "The following warning(s) were raised:\n" + str(retval)
warnings = str(retval)
ajaxdict["data"] = "The following warning(s) were raised:\n" + warnings.replace('\n', '<br />')
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
ajaxdict["data"] = "The following warning(s) were raised:\n" + warnings.replace('\n', '<br />')
ajaxdict["data"] = "The following warning(s) were raised:\n" + warnings.replace('\n', '<br />\n')

src/app/core.py Outdated
@@ -327,7 +328,8 @@ def license_validate_helper(request):
""" If any warnings are returned """
if (request.is_ajax()):
ajaxdict["type"] = "warning"
ajaxdict["data"] = "The following warning(s) were raised: " + str(retval)
warnings = str(retval)
ajaxdict["data"] = "The following warning(s) were raised:\n" + warnings.replace('\n', '<br />')
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
ajaxdict["data"] = "The following warning(s) were raised:\n" + warnings.replace('\n', '<br />')
ajaxdict["data"] = "The following warning(s) were raised:\n" + warnings.replace('\n', '<br />\n')

src/app/core.py Outdated
@@ -519,7 +521,8 @@ def license_convert_helper(request):
else :
if (request.is_ajax()):
ajaxdict["type"] = "warning"
ajaxdict["data"] = "The following warning(s) were raised by "+ myfile.name + ": " + str(warnings)
warnings = str(warnings)
ajaxdict["data"] = "The following warning(s) were raised by "+ myfile.name + ": " + warnings.replace('\n', '<br />')
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
ajaxdict["data"] = "The following warning(s) were raised by "+ myfile.name + ": " + warnings.replace('\n', '<br />')
ajaxdict["data"] = "The following warning(s) were raised by "+ myfile.name + ": " + warnings.replace('\n', '<br />\n')

@BassCoder2808
Copy link
Contributor Author

Thanks @BassCoder2808 - just a minor suggested change to format the raw HTML a bit better

Hi @goneall I have made the changes, let me know if anything else needs to be changed

Copy link
Member

@goneall goneall left a comment

Choose a reason for hiding this comment

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

@goneall goneall merged commit bec5b78 into spdx:main Jul 28, 2023
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