-
Notifications
You must be signed in to change notification settings - Fork 1
Add results endpoint to download a static html file #10
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
base: main
Are you sure you want to change the base?
Conversation
cybernop
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you format all files using black from the recommanded extensions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The html file is missing the mapping information. Maybe this is related to the css definition not being included in the file?
|
And what was the reason for you to create a fork for working on the changes instead of creating a simple branch? |
cybernop
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also rebase on the latest state of main to enforce checks
I did not have the necessary rights, so I was forced to create a fork |
cybernop
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have to validate some information
cybernop
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix the type mismatch and also remove commented out code :)
cybernop
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you fix the small detail i commented? The rest looks good :)
| if not self.__projs: | ||
| path = self.projs_dir | ||
| if path.is_dir() and (path / "config.json").exists(): | ||
| try: | ||
| self.__projs[path.name] = Project(path) | ||
| except ValidationError as e: | ||
| logger.error(e.errors()) | ||
| raise e | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make sure the this only accepts a directory where the projects are placed like before. Handling a single project for the CLI should be handled differently.
Maybe you can skip the ProjectManager completely and just use a project directly.
This pull request creates a new endpoint that returns the mapping structure as a static html page.
The function that generates the html file has also been migrated to the new data model.