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.

Sign up for GitHub

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

Jump to bottom

feat: technical review #11

Merged
merged 38 commits into from
Feb 28, 2024
Merged

feat: technical review #11

merged 38 commits into from
Feb 28, 2024

Conversation

Revathyvenugopal162
Copy link
Contributor

@Revathyvenugopal162 Revathyvenugopal162 commented Feb 20, 2024 •
edited
Loading

  • add licenec headed
  • update pre-commit
  • add unit test for pyproject projects
  • improve tests
  • improve docs
  • remove unwanted funcs from con.py
  • add required files

@github-actions github-actions bot added the enhancement New features or code improvements label Feb 20, 2024
@Revathyvenugopal162 Revathyvenugopal162 force-pushed the feat/tech-review branch 3 times, most recently from fe3b33a to 43bdb38 Compare February 21, 2024 09:07
@Revathyvenugopal162 Revathyvenugopal162 marked this pull request as ready for review February 21, 2024 11:14
ansjhenry
ansjhenry approved these changes Feb 21, 2024
View reviewed changes
tests-pytest/action.yml Outdated Show resolved Hide resolved
@ansjhenry ansjhenry mentioned this pull request Feb 21, 2024
Closed
ansjhenry
ansjhenry reviewed Feb 22, 2024
View reviewed changes
get-scade-python/action.yml
@@ -8,9 +8,8 @@ description: >
inputs:
scade-dir:
description: >
Ansys SCADE Installation directory, e.g.::
Copy link
Collaborator

Choose a reason for hiding this comment

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

FYI, I tried the usage of back ticks but the rendering is not good, the path is split on two lines, that's why I've chosen the inlining of Python-like code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

let me check this and try to resolve

requirements/requirements_tests.txt Show resolved Hide resolved
.github/CODEOWNERS Show resolved Hide resolved
@Revathyvenugopal162
Copy link
Contributor Author

Revathyvenugopal162 commented Feb 27, 2024 •
edited
Loading

Additional changes after review,

  • Change in license file
  • make all actions and its descriptions from
  • remove unused fns from conf.py

jorgepiloto
jorgepiloto approved these changes Feb 28, 2024
View reviewed changes
Copy link
Member

@jorgepiloto jorgepiloto left a comment

Choose a reason for hiding this comment

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

Amazing. This looks great. Approving 👍🏼

@ansjhenry ansjhenry merged commit 477c121 into main Feb 28, 2024
13 checks passed
@ansjhenry ansjhenry deleted the feat/tech-review branch February 28, 2024 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ansjhenry ansjhenry ansjhenry approved these changes

@jorgepiloto jorgepiloto jorgepiloto approved these changes

Assignees
No one assigned
Labels
enhancement New features or code improvements
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Revathyvenugopal162 @jorgepiloto @ansjhenry