Skip to content

Conversation

@yungyuc
Copy link
Member

@yungyuc yungyuc commented Jul 17, 2022

Remove the primitive C++ code for running the viewer pytext code for an application to the new module modmesh.apputil. A new class AppEnvironment is defined to hold the symbols for the application, so that the symbols will not scatter in the interpreter __main__.

Each run of the applications will create a new app environment, which is collected in the list modmesh.apputil.environ.

Also in this commit: Rename FIXME to TODO.

ref #100

Remove the primitive C++ code for running the viewer pytext code for an application to the new module `modmesh.apputil`.  A new class `AppEnvironment` is defined to hold the symbols for the application, so that the symbols will not scatter in the interpreter `__main__`.

Each run of the applications will create a new app environment, which is collected in the list `modmesh.apputil.environ`.

Also in this commit: Rename `FIXME` to `TODO`.
@yungyuc yungyuc added the refactor Change code without changing tests label Jul 17, 2022
@yungyuc yungyuc self-assigned this Jul 17, 2022
@yungyuc yungyuc marked this pull request as ready for review July 17, 2022 08:10
* Run all supported platforms (ubuntu 20.04 and 22.04, macos 11 and 12) with only Release flavor and without clang-tidy (which takes a long of runtime).
* Add a new CI workflow `modmesh_lint` to only run clang-tidy and flake8 (linter) with Debug flavor and the latest platform (ubuntu 22.04 and macos 12).
* Fix the directory name of `modmesh_format`.
* Update `modmesh_install` to use the latest platform (ubuntu 22.04 and macos 12).
path:
- 'include'
- 'src'
- 'cpp'
Copy link
Member Author

Choose a reason for hiding this comment

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

We ran the workflow without checking anything for a while :)

strategy:
matrix:
os: [ubuntu-20.04, macos-11]
os: [ubuntu-22.04, macos-12]
Copy link
Member Author

Choose a reason for hiding this comment

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

Use latest platform.

- name: dependency (ubuntu)
if: ${{ startsWith(matrix.os, 'ubuntu') }}
run: |
VERSION_ID=$(bash -c 'source /etc/os-release ; echo $VERSION_ID')
Copy link
Member Author

Choose a reason for hiding this comment

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

Align from other workflows.


strategy:
matrix:
os: [ubuntu-22.04, macos-12]
Copy link
Member Author

Choose a reason for hiding this comment

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

Run clang-tidy and flake8 only with the latest platforms and debug flavor.

run: |
make buildext \
VERBOSE=1 BUILD_QT=OFF USE_CLANG_TIDY=ON \
VERBOSE=1 BUILD_QT=OFF USE_CLANG_TIDY=OFF \
Copy link
Member Author

Choose a reason for hiding this comment

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

Turn off clang-tidy for pytest workflow.

make pytest \
VERBOSE=1 BUILD_QT=OFF USE_CLANG_TIDY=ON \
CMAKE_BUILD_TYPE=${{ matrix.cmake_build_type }} \
CMAKE_ARGS="-DPYTHON_EXECUTABLE=$(which python3)"
Copy link
Member Author

Choose a reason for hiding this comment

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

Turn off flake8 for pytest workflow.

}; /* end struct StaticMeshConstant */

// FIXME: StaticMeshBC may use polymorphism.
// TODO: StaticMeshBC may use polymorphism.
Copy link
Member Author

Choose a reason for hiding this comment

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

FIXME should be a reminder before checking in. TODO may live longer.


#include <modmesh/spacetime/base_spacetime.hpp>
#include "modmesh/math.hpp"
#include <modmesh/math.hpp>
Copy link
Member Author

Choose a reason for hiding this comment

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

Fix typo.

/* TODO: parse arguments */

// Set up menu.
/* TODO: Menu setup may be moved to Python */
Copy link
Member Author

Choose a reason for hiding this comment

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

@yanagiragi Moving this to Python can make it easier to maintain.

The local namespace of the application.
"""
def __init__(self):
self.locals = {
Copy link
Member Author

Choose a reason for hiding this comment

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

Give the environment a local namespace.

@yungyuc yungyuc merged commit c450eaf into solvcon:master Jul 17, 2022
@yungyuc yungyuc deleted the refactor/app-environment branch July 17, 2022 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Change code without changing tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant