Skip to content

Clean up code and the repo#1

Open
bepzi wants to merge 4 commits into
tipei:masterfrom
bepzi:master
Open

Clean up code and the repo#1
bepzi wants to merge 4 commits into
tipei:masterfrom
bepzi:master

Conversation

@bepzi

@bepzi bepzi commented Oct 23, 2019

Copy link
Copy Markdown

This pull request:

  • Deletes and blacklists the build outputs, which don't need to be in the repo
  • Applies clang-format to all the source code, to fix the lack of a consistent code style
  • Re-uses the current terminal when synthesizing, rather than spawning a new instance of gnome-terminal

bepzi added 4 commits October 22, 2019 22:21
Otherwise, DISSCO cannot be compiled without first running `premake4
clean`. These files don't need to be in the repo anyways.

Add a `.gitignore` that prevents these build outputs from being
re-committed to the repo.
Instead, re-use `stdout` in the current terminal. We do this because
not all machines have `gnome-terminal` installed, nor is it convenient
to have to close the spawned instances of `gnome-terminal` each time
you synthesize your piece.

This is also less prone to breaking; `gnome-terminal` has deprecated
the -e flag and that behavior is not standard across terminal
emulators.

@cjkrieseSF cjkrieseSF left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This looks like a good change

@cjkrieseSF cjkrieseSF left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No harm in this

@cjkrieseSF cjkrieseSF left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Agreed

@cjkrieseSF

Copy link
Copy Markdown

Generally, good changes, but conflicts need cleaning before merge.

@bepzi

bepzi commented Mar 21, 2020

Copy link
Copy Markdown
Author

Given that I applied clang-format to most of the files, fixing these merge conflicts will require re-doing a lot of that work, and I'm no longer invested in this pull request to do it again myself.

If you'd still like my contributions, I'd recommend you cherry-pick them or re-do them. My fork (https://github.com/bepzi/DISSCO-2.0.2) has several branches that might be of interest to you:

  • cmake-port is a port of the build system to CMake 3, rather than premake.

  • code-cleanup is a series of bugfixes and lints that improve the health of the codebase (fixing usages of uninitialized memory, etc.) It is independent of the cmake-port branch, and still uses the premake build system.

  • gtk3-port was my attempt at porting the GUI to GTK+3, but I never quite got it to work. The initial steps are there, though.

My recommendation to you would be:

  1. Re-do my work in this pull request. Apply clang-format, blacklist the build outputs, and fix the usage of gnome-terminal.

  2. Go through my code-cleanup branch and apply anything that seems valuable to you. Without these changes, I was unable to use DISSCO on any Linux machine other than Prof. Tipei's Ubuntu machines without regular segfaulting. This branch even fixes a few segfaults that occurred on those Ubuntu machines!

  3. Consider applying my cmake-port branch, and discontinue using premake.

  4. At some point in the far future, consider beginning a migration to GTK+3.

tipei pushed a commit that referenced this pull request Nov 3, 2024
@passyur passyur force-pushed the master branch 2 times, most recently from f1d0dbc to 2fb56bc Compare May 7, 2025 22:53
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.

2 participants