Skip to content

Add emails and urls to HTML output - #2539

Merged
pombredanne merged 2 commits into
aboutcode-org:developfrom
sritasngh:html_temp
Aug 13, 2021
Merged

Add emails and urls to HTML output#2539
pombredanne merged 2 commits into
aboutcode-org:developfrom
sritasngh:html_temp

Conversation

@sritasngh

@sritasngh sritasngh commented Jun 5, 2021

Copy link
Copy Markdown
Contributor

Add emails and urls in converted_infos. Earlier emails and urls were excluded from infos so these informations weren't fed into html template. Add holders, authors, emails and urls information in html template.

Signed-off-by: Sarita Singh saritasingh.0425@gmail.com
Co-authored-by: Avinal Kumar avinal.xlvii@gmail.com

Partially Solves #1359

Tasks

  • Reviewed contribution guidelines
  • PR is descriptively titled 📑 and links the original issue above 🔗
  • Tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR
    Run tests locally to check for errors.
  • Commits are in uniquely-named feature branch and has no merge conflicts 📁

@pombredanne pombredanne left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looking good!
Could you add tests too, and a CHANGELOG entry?

@sritasngh
sritasngh requested a review from pombredanne June 6, 2021 19:58

@avinal avinal left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A rewording suggestion

Comment thread CHANGELOG.rst Outdated
@pombredanne pombredanne changed the title Add emails and urls Add emails and urls to HTML output Jun 15, 2021

@pombredanne pombredanne left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is looking good but I am not sure that we have a tests that this is effectively working?
You would need to assert something that proves that the data is effectively included.

@sritasngh
sritasngh marked this pull request as draft August 9, 2021 11:16
@sritasngh
sritasngh marked this pull request as ready for review August 9, 2021 17:17
@sritasngh
sritasngh requested a review from pombredanne August 9, 2021 17:17
Comment thread tests/formattedcode/test_output_templated.py Outdated

@pombredanne pombredanne left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM overall. Thanks!
There is one tiny nit in tests.... but we could merge without this

sritasngh and others added 2 commits August 13, 2021 15:48
Add emails and urls in converted_infos. Add holders, authors, emails and
urls information in html template.

Signed-off-by: Sarita Singh <saritasingh.0425@gmail.com>
Co-authored-by: Avinal Kumar <avinal.xlvii@gmail.com>
Signed-off-by: Sarita Singh <saritasingh.0425@gmail.com>

Co-authored-by: Avinal Kumar <avinal.xlvii@gmail.com>
@pombredanne

Copy link
Copy Markdown
Member

Thanks!
I will merge as soon as the test runs are completed.

Comment thread tests/formattedcode/test_output_templated.py
@pombredanne

pombredanne commented Aug 13, 2021

Copy link
Copy Markdown
Member

@itssingh @avinal Thank you ++, merging.

@pombredanne
pombredanne merged commit 41d2d5b into aboutcode-org:develop Aug 13, 2021
@avinal

avinal commented Aug 13, 2021

Copy link
Copy Markdown
Contributor

Thanks @pombredanne 😊

@sritasngh

Copy link
Copy Markdown
Contributor Author

Thanks @pombredanne :)

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