Skip to content

Fixes #3289 - Add support for nested ul in browser details#3290

Merged
karlcow merged 4 commits intomasterfrom
issues/3289/1
Apr 22, 2020
Merged

Fixes #3289 - Add support for nested ul in browser details#3290
karlcow merged 4 commits intomasterfrom
issues/3289/1

Conversation

@miketaylr
Copy link
Copy Markdown
Member

(This will be used by the GPU data, if it exists)

I feel like Karl will have some opinions on this one, and it's cosmetic, so it can wait for him to come back.

(This will be used by the GPU data, if it exists)
@miketaylr miketaylr requested a review from karlcow April 16, 2020 18:49
Copy link
Copy Markdown
Member

@karlcow karlcow left a comment

Choose a reason for hiding this comment

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

@miketaylr

  1. maybe change the name
  2. double check that we are not doing at two places the same thing.

Comment thread webcompat/helpers.py Outdated
Mike Taylor added 3 commits April 22, 2020 14:50
@miketaylr
Copy link
Copy Markdown
Member Author

OK, @karlcow. I refactored / renamed things a bit -- hopefully less confusing.

r? @karlcow

Copy link
Copy Markdown
Member

@karlcow karlcow left a comment

Choose a reason for hiding this comment

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

Thanks @miketaylr
more elegant!

Comment thread webcompat/helpers.py
def get_list_items(val_dict):
"""Return list items (<li>) from the passed in list ([])."""
rv = ''.join(['<li>{k}: {v}</li>'.format(k=k, v=get_serialized_value(v))
for k, v in list(val_dict.items())])
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.

The code was inherited from probably the conversion from python 2 to python 3. IN this specific case val_dict.items() is a <class 'dict_items'> and is an iterable, which makes the list() wrapper unecessary

@karlcow karlcow merged commit ccedceb into master Apr 22, 2020
@karlcow karlcow deleted the issues/3289/1 branch April 22, 2020 20:59
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.

2 participants