infrastructure/extensions-website
4
8
Fork 10
Code Issues 20 Pull Requests 2 Projects Releases Wiki Activity

Improve validation messages #196

Merged
Dalai Felinto merged 7 commits from pr-error-messages into main 2024-07-01 15:22:55 +02:00
Conversation 8 Commits 7 Files Changed 5 +13 -12
1 changed files with 13 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 4fe07c2204 - Show all commits

25
files/utils.py
View File

@ -172,13 +172,21 @@ def find_forbidden_filepaths(file_list):
def validate_file_list(toml_content, manifest_filepath, file_list):
"""Check the files in in the archive against manifest."""
error_codes = []
type_slug = toml_content['type']
found_forbidden_filepaths = find_forbidden_filepaths(file_list)
# Special treatment for Mac, so the same problem (__MACOSX folders) doesn't lead to two errors showing.
if type_slug == 'theme' and '__MACOSX/' not in found_forbidden_filepaths:
found_forbidden_filepaths = find_forbidden_filepaths(file_list)
if found_forbidden_filepaths:
error_codes.append(
{
dfelinto marked this conversation as resolved Outdated
Oleg-Komarov commented 2024-07-01 15:11:03 +02:00
Outdated
Review
Copy Link

let's restructure this check a bit to prevent possible confusion in the future:

  • now the whole theme branch is skipped, while we mean to skip only a particular check
  • if in the future more theme-related checks are added, we would likely want them to run

i.e. let's move the and '__MACOSX/' not in found_forbidden_filepaths part to the if len(list(theme_xmls)) != 1 line

let's restructure this check a bit to prevent possible confusion in the future: - now the whole theme branch is skipped, while we mean to skip only a particular check - if in the future more theme-related checks are added, we would likely want them to run i.e. let's move the `and '__MACOSX/' not in found_forbidden_filepaths` part to the `if len(list(theme_xmls)) != 1` line
Oleg-Komarov commented 2024-07-01 15:14:17 +02:00
Outdated
Review
Copy Link

and it's better to move the found_forbidden_filepaths logic closer to the variable declaration - i.e. let's move the whole block up, not just the variable declaration

and it's better to move the `found_forbidden_filepaths` logic closer to the variable declaration - i.e. let's move the whole block up, not just the variable declaration
'code': 'forbidden_filepaths',
'params': {'paths': ', '.join(found_forbidden_filepaths)},
}
)
type_slug = toml_content['type']
if type_slug == 'theme':
theme_xmls = filter_paths_by_ext(file_list, '.xml')
if len(list(theme_xmls)) != 1:
# Special treatment for Mac, so the same problem (__MACOSX folders)
# doesn't lead to two errors showing.
if len(list(theme_xmls)) != 1 and '__MACOSX/' not in found_forbidden_filepaths:
error_codes.append('missing_or_multiple_theme_xml')
elif type_slug == 'add-on':
# __init__.py is expected to be next to the manifest
@ -195,13 +203,6 @@ def validate_file_list(toml_content, manifest_filepath, file_list):
error_codes.append(
{'code': 'missing_wheel', 'params': {'path': expected_wheel_path}}
)
if found_forbidden_filepaths:
error_codes.append(
{
'code': 'forbidden_filepaths',
'params': {'paths': ', '.join(found_forbidden_filepaths)},
}
)
return error_codes