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

Fix style and mypy errors #6475

Closed

Conversation

sunnycarter
Copy link
Contributor

Various bugs had occurred due to merge errors and not running mypy.

Jamieparsons and others added 25 commits July 4, 2023 12:45
# Main changes

## Make methods relying on self._tmp_dir private
- `self._tmp_dir` is only available in the context of calling `generate_nfd()`, so methods relying on `self._tmp_dir` should be private

## Use pathlib.Path rather than os file operations
- Provides clearer and stronger typing than passing `str`s around
- Adds some handy utility functions

## Variable renaming for clarity
- E.g. consistently use 'directory' / 'dir' (rather than mix with 'folder')
- Obvs somewhat subjective, but as someone new to most of this code, the changes made sense to me

## Add nfd_bicep_path as abstract property on NFDGenerator
- We rely on it when calling the concrete implementations
- Also use ABC rather than raise NotImplementedError

## Miscellaneous style updates to keep `azdev style aosm` happy
- isort
- black
Add unit tests for build and generate config.
@azure-client-tools-bot-prd
Copy link

Hi @sunnycarter,
Please write the description of changes which can be perceived by customers into HISTORY.rst.
If you want to release a new extension version, please update the version in setup.py as well.

@yonzhan
Copy link
Collaborator

yonzhan commented Jul 5, 2023

Thank you for your contribution! We will review the pull request and get back to you soon.

@sunnycarter
Copy link
Contributor Author

Put this on the wrong repo sorry

@sunnycarter sunnycarter closed this Jul 5, 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.

None yet

9 participants