Skip to content

Conversation

@RobPasMue
Copy link
Member

@RobPasMue RobPasMue commented Aug 23, 2023

Addressing minimum requirements and general enhancements. Listing changes:

  • Moving orphan scripts to scripts folder 7f4b061
  • Add missing license headers to Python scripts 9b19df8
  • Updating pre-commit hooks and related changes: 62aca01 5f4b879 ed99e08
  • Changing Docker related files to docker folder 58c827b
  • Renaming "Ansys additive" to "Ansys Additive" since it is an Ansys product 0deceb7
  • Limiting major Python version to be 3.X fe0e0dc
  • Adding missing metadata to pyproject.toml 3914c7d
  • Changing versioning scheme from MAJOR.MINOR.PATCH.devX to MAJOR.MINOR.PATCH/devX - no need for extra element since dev versions should be the minor branches 04048df
  • Change documentation tox output to common location at doc/_build/html 9d5f1ee
  • Improving general workflow for CI/CD 85ce01f 9f27066
  • Proper TODO statements to be removed after going public (related to linkcheck) 31ba587
  • Fixing source code docstrings wrongly formatted 6fba567
  • Use Enum for classes only holding constants b819e7f 8dc6180 f2c91ff

There are more changes to come though since the ansys-api-additive package has to be made public (and reviewed). At that point, we will have to remove all references to the private PyPI. This is my next step.

However, there are also a couple of major changes and suggestions that I have noted... proposing them here before actually performing the changes since they will impact most of the codebase

  • Change package name from ansys-additive to ansys-additive-core (package rename, folder structure, import statements... a lot of affected files)
  • Suggestion - dependencies for testing should be pinned. See other libraries. However this is not compulsory - it's just to ease the maintenance. 62299d6
  • Suggestion - changing to autoapi rather than pure autosummary (docs) to avoid missing out on documentation for classes, methods etc. 005a9ec
  • Suggestion - Removing star imports in favor of actual imports. See example discussion. d5669a4

Please @pkrull-ansys - comment on the above suggestions and changes made before proceding to perform more changes.

@RobPasMue RobPasMue self-assigned this Aug 23, 2023
@github-actions github-actions bot added documentation Improvements or additions to documentation maintenance Package and maintenance related testing enhancement New features or code improvements labels Aug 23, 2023
@github-actions github-actions bot added testing and removed testing labels Aug 23, 2023
@RobPasMue
Copy link
Member Author

I've seen there are some issues on the tests, coming from the Enum class change I did. I will solve those asap.

@github-actions github-actions bot removed the testing label Aug 23, 2023
@github-actions github-actions bot removed the testing label Aug 25, 2023
@github-actions github-actions bot added testing and removed testing labels Aug 25, 2023
@github-actions github-actions bot added testing and removed testing labels Aug 26, 2023
@pkrull-ansys pkrull-ansys added this pull request to the merge queue Aug 27, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Aug 27, 2023
@RobPasMue RobPasMue added this pull request to the merge queue Aug 27, 2023
@RobPasMue RobPasMue removed this pull request from the merge queue due to a manual request Aug 27, 2023
@RobPasMue RobPasMue added this pull request to the merge queue Aug 27, 2023
Merged via the queue into main with commit 36fe501 Aug 27, 2023
@RobPasMue RobPasMue deleted the feat/tech-review branch August 27, 2023 09:00
@MaxJPRey
Copy link
Contributor

Thanks @RobPasMue for this thorough review m.

@RobPasMue RobPasMue mentioned this pull request Aug 30, 2023
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation enhancement New features or code improvements maintenance Package and maintenance related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants