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

Add CNF NFD generation to build command #8

Merged
merged 55 commits into from
May 19, 2023

Conversation

jddarby
Copy link
Owner

@jddarby jddarby commented May 18, 2023


This checklist is used to make sure that common guidelines for a pull request are followed.

Related command

General Guidelines

  • Have you run azdev style <YOUR_EXT> locally? (pip install azdev required)
  • Have you run python scripts/ci/test_index.py -q locally?

For new extensions:

About Extension Publish

There is a pipeline to automatically build, upload and publish extension wheels.
Once your pull request is merged into main branch, a new pull request will be created to update src/index.json automatically.
You only need to update the version information in file setup.py and historical information in file HISTORY.rst in your PR but do not modify src/index.json.

@jordlay jordlay merged commit 13f72f4 into add-aosm-extension May 19, 2023
Copy link
Collaborator

@sunnycarter sunnycarter left a comment

Choose a reason for hiding this comment

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

On the whole looks very good. Most of my comments are about the need to explain what is going on rather than coding errors.

I have run azdev aosm style and it throws a load of errors, most of which should be fixed. (I always get an error with flake8 failing to run with ValueError: Error code '#' supplied to 'ignore' option does not match '^[A-Z]{1,3}[0-9]{0,3}$' which I never got to the bottom of, but there are plenty other errors.)

Personally, I cheated a bit for fixing them up and installed python-static-checks into the az aosm virtual environment. This is a tool used in quicksilver projects, and running python-static-checks fmt fixed a load of the errors for me automatically. I've written in setup.md how you install it.

Copy link
Collaborator

@sunnycarter sunnycarter left a comment

Choose a reason for hiding this comment

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

On the whole looks very good. Most of my comments are about the need to explain what is going on rather than coding errors.

I have run azdev aosm style and it throws a load of errors, most of which should be fixed. (I always get an error with flake8 failing to run with ValueError: Error code '#' supplied to 'ignore' option does not match '^[A-Z]{1,3}[0-9]{0,3}$' which I never got to the bottom of, but there are plenty other errors.)

Personally, I cheated a bit for fixing them up and installed python-static-checks into the az aosm virtual environment. This is a tool used in quicksilver projects, and running python-static-checks fmt fixed a load of the errors for me automatically. I've written in setup.md how you install it.


# only add the parameter name (e.g. from {deployParameter.zone} only param = zone)
param = v.split(".", 1)[1]
param = param.split("}", 1)[0]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Anything we should do here to guard against whoever wrote the file putting too many spaces in somewhere?

Copy link
Collaborator

Choose a reason for hiding this comment

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

for Jacob

@jordlay jordlay deleted the jl/add-cnf-generation branch June 1, 2023 09:31
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.

4 participants