Restructured project, added gradle, JUnit, logger, and more #83

RubbaBoy · 2020-07-06T06:54:12Z

I found this project and it seemed to be the only one that did what this does without any native binaries, which is awesome. The one biggest drawback I noticed in the project was the lack of Maven or Gradle, which makes it a lot more troublesome to work with.

This PR restructures the project a bit, adding Gradle (And upgrading to Java 11, the current LTS). It improves the tests as well, using JUnit 5 while retaining the old messages and functionality. The test/java/com/jysn/examples directory contained no tests but standalone examples, so I have moved them into an examples module for various reasons. I also added SLF4J, instead of using System.out.println for everything, or using the Java logger which popped up here and there. A /DEVELOPING.md folder was also added for ease of use with some commands, which can be removed if needed. IDE-specific related folders/files were also removed/ignored via .gitignore to promote building by Gradle, to allow for easier cross-machine compilation and usage. In addition, nothing related to code functionality has been changed.

These changes were mainly meant for my own sanity and to use this in future projects, if you choose not to accept this that's totally fine, it's a lot of changes to a pretty old project. I believe something along this path would be required if you were to put this on something like Maven Central or GitHub Packages though.

Let me know if you would like any changes and I'll try my best.

Added Gradle (and removed ant), modernized testing via the JUnit framework, moved standalone examples from the tests directory to a separate module, removed sparsely used Java logger and replaced it with SLF4J. More work could be done, however this is a great start to greatly improving the health of the codebase.

philburk · 2020-07-06T14:28:09Z

@RubbaBoy - Thanks for this PR. I added an Issue last week for putting JSyn on Maven. #82
Was this a coincidence.

There are a ton of great changes in here that I want to use. I will go through the files and figure out what parts I can keep. I may have to break this up into separate commits.

RubbaBoy · 2020-07-06T14:32:14Z

Yeah, I had the intention of adding Maven repository support when I found the repo and saw the issue while I was in the process. And I did do quite a lot in just that one commit, it would be nice to split it up.

philburk

I have started my review but only got about a third of the way through.
Generally I think your changes are great. This is a nice upgrade for JSyn.
I can move my Eclipse code outside JSyn. That is OK.

My review comments may sound negative but that is mostly just resistance to change. There are a huge number of changes to something that has been stable for a long time. I really appreciate your changes and see them as a gift.

My concerns are mainly about the impact of the added dependencies.

Also do any of the language upgrades, eg. use of "var" and lambdas, prevent exporting of a Java 1.8 executable JAR file? That is crucial for distributing JSyn apps.

I will download your changes and see if I run into any problems.

android/com/jsyn/devices/android/AndroidAudioForJSyn.java

philburk · 2020-07-06T14:36:13Z

build.gradle

+    id 'java'
+    id 'application'
+    id 'maven-publish'
+    id 'com.github.johnrengelman.shadow' version '6.0.0'


I have resisted adding dependencies to JSyn. Why is this needed?

The first three plugins are official Gradle plugins that are developed with the build tool, I would be hesitant to even call them dependencies. The java plugin is required to compile Java programs (And serves for the base of other Java-related plugins in the build tool). The application plugin is required to run it easily via Gradle, and the maven-publish plugin is required for publishing to both your local and any remote maven repositories. The shadow plugin is not created and maintained by Gradle, however is a mature plugin to assist in the creation and building of jars, giving more options to do so. I can probably remove this one, however, the java and maven-publish are the minimum required by Gradle to use with local/remote maven repositories.

philburk · 2020-07-06T14:42:50Z

build.gradle

+    mavenLocal()
+}
+
+dependencies {


One way that JSyn has survived since 1997 is by reducing dependencies. One of the main reasons that a software project dies is because a dependency becomes unavailable.
Are these new dependencies necessary or just handy?
How standard are these dependencies? Are they widely used?
Are they well supported? Will they always be available?
I suppose that if the dependencies are limited to the unit tests or build system then JSyn can still survive.

The only dependencies used are the JUnit framework (One of, if not the biggest and most widely used testing frameworks) which includes easy testing and integration among all major IDEs. The other is the logging API (discussed outside of the thread), along with the local jar of course. These are both used by hundreds of thousands of projects (If not more), are not planned to have support stopped on any of them, and are available forever on the Maven Central repository. (I will be looking over your other comments later today as well)

philburk · 2020-07-06T14:55:15Z

build.xml

@@ -1,66 +0,0 @@
-<project name="JSynProject" default="dist" basedir=".">


Gradle seems to be winning over Ant.

But some people might be relying on this. Let's deprecate it, warn people not to use it, but keep it until the transition to gradle is complete. That will help people transition.

The README.md says to use ant. it fails because build.xml is deleted. So this is broken.

examples/src/main/java/com/jsyn/examples/ChebyshevSong.java

philburk · 2020-07-06T15:02:17Z

examples/src/main/java/com/jsyn/examples/AudioPassThrough.java

@@ -51,7 +54,7 @@ private void test() {

        // We only need to start the LineOut. It will pull data from the LineIn.
        lineOut.start();
-        System.out.println("Audio passthrough started.");
+        LOGGER.debug("Audio passthrough started.");


These example programs were meant to be run from the command line and print messages to the Terminal. System.out.println() is a standard way of doing that.

Where will these logs appear?

org.slf4j.Logger is using System.out as fallback.

When I run the PlayNotes.java example it does not print the messages unless I revert to using System.out.

philburk · 2020-07-06T15:04:48Z

examples/src/main/java/com/jsyn/examples/ListAudioDevices.java

@@ -34,10 +38,10 @@ public static void main(String[] args) {
            int maxOutputs = audioManager.getMaxInputChannels(i);
            boolean isDefaultInput = (i == audioManager.getDefaultInputDeviceID());
            boolean isDefaultOutput = (i == audioManager.getDefaultOutputDeviceID());
-            System.out.println("#" + i + " : " + deviceName);
-            System.out.println("  max inputs : " + maxInputs
+            LOGGER.debug("#" + i + " : " + deviceName);


This program is supposed to list devices to the Terminal. Does it still do that?

Yes, functionality remains unchanged other than adding the timestamp to the message via the logging implementation (I double-checked this as well)

Adding a timestamp to each line would be really annoying. Imagine entering 'df' in Linux and seeing a timestamp on every line. These are not logs. They are the output of a command line program.

examples/src/main/java/com/jsyn/examples/PlayFunction.java

src/main/java/com/jsyn/apps/AboutJSyn.java

RubbaBoy · 2020-07-06T18:32:52Z

That's completely fine, I wouldn't expect someone with an old project like this to accept all changes without question.

I'd like to bring logging out in a separate discussion, as it will most likely be brought up more. I added logging via SLF4J, as it is one of the most widely used and mature logging frameworks, allowing for finer control over output compared to standard System.out.println. (I forgot to make Log4J test-only, I will add a commit for that later today). It's just an API, with the Log4J implementation just in examples and tests. Using an implementation such as the provided one, it still prints everything to console (by default with just the timestamp and the log level), with the option to pipe it to a file or anything else that is needed. It also has Android bindings that are used in many projects, which take advantage of the default Android logging package without tailoring your code specifically to Android. I can definitely remove the logging if you think it isn't worth the hassle though.

RubbaBoy · 2020-07-07T04:34:40Z

In reference to your Java 8 comment, I didn't realize it would have such an impact on usage. Lambdas were introduced in Java 8 so adding them is only removing verbosity from the code, however, using var would break compatibility.

Includes a minor logging fix

philburk · 2020-08-08T23:16:01Z

DEVELOPING.md

+### Run The Demo
+
+```
+./gradlew run


I tried this and got a "Permission denied" error.
After: chmod +x ./gradlew
then it worked.

philburk · 2020-08-08T23:19:47Z

DEVELOPING.md

+### Javadoc generation
+
+```
+./gradlew javadoc


Add a comment to look for output in "build/docs/javadoc".

philburk

I would love to have 90% of these changes. I was able to use the new JAR file with an old project and it worked fine. The SLF4J stubs worked. But there are a few problems.
I had to add SLF4J to my Eclipse project to get it to work. I can live with that. I will just need to update the docs on how to use Eclipse with JSyn.

The main problem is that the examples no longer print to the console as intended. They are meant to be educational but now they print nothing. Can you you please restore the use of System.out.println() in the examples package?

philburk · 2020-08-08T23:24:28Z

DEVELOPING.md

+./gradlew javadoc
+```
+
+### Building Jar


Add a comment to look in build/libs

philburk

I keep trying to use the new repository for my work. I cannot get the examples to run in Eclipse. I would love to support Gradle and Maven. But there are just too many other changes in this pull request that break things for me. Thanks so much for offering it. But I will not be merging this pull request.

DivineThreepwood

I would really love to have this PR merged.
I understand your critical comments regarding adding dependencies to this lib. However, I totally recommend to use org.slf4j.Logger instead sticking to the old fashing System.out since it offers much more flexibility regarding log filtering and multi architecture support such as Android.

I am using jsyn already a couple of years but the main reasons to replace it soon is the missing maven and logging support. So please merge the PR and if you need help to host the lib at central maven just let me know!

philburk · 2020-10-30T18:08:26Z

I totally recommend to use org.slf4j.Logger instead sticking to the old fashing System.out

I am fine with removing System.out for logging. Those were just some debugging hacks that I left commented out.
I will probably just remove those lines completely.

But I deliberately use System.out in the console apps because I want to print to the console. I am not logging.
Those apps are now broken because they no longer print.

philburk · 2020-10-30T19:13:57Z

I merged these change. I really wanted the Maven/gradle changes. You have also made numerous cleanup changes
and test improvements. Thank you for those changes.

It is very unfortunate that your SLF4J logger changes will break JSyn for many people.
So I will be going through and removing as much logging as possible.
You also broke the command line apps that used to print to the console.
These issues were never resolved so I will have to fix these problems.
I appreciate the work you you are offering. I just hope that in the future you will split
your changes into smaller separate merge requests so that they can be considered separately.
This all-or nothing approach made it very difficult to accept these changes.

RubbaBoy added 2 commits July 6, 2020 02:33

Removed .settings/

37237a7

philburk requested changes Jul 6, 2020

View reviewed changes

RubbaBoy added 2 commits July 7, 2020 01:13

Downgraded to Java 8

4ee20b0

Includes a minor logging fix

Removed shadow jar

52dec56

This was referenced Jul 15, 2020

Add gradle or maven workflow dhowe/rita4j#14

Closed

Add gradle publishing workflow dhowe/rita4j#20

Closed

Replace System.out with logger dhowe/rita4j#21

Open

Upgrade Java for functions dhowe/rita4j#23

Closed

Removed slf4j for builds

ff7d520

philburk reviewed Aug 8, 2020

View reviewed changes

DEVELOPING.md

### Javadoc generation

```

./gradlew javadoc

Copy link

Owner

philburk Aug 8, 2020

Choose a reason for hiding this comment

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

Add a comment to look for output in "build/docs/javadoc".

philburk requested changes Aug 9, 2020

View reviewed changes

DEVELOPING.md

./gradlew javadoc

```

### Building Jar

Copy link

Owner

philburk Aug 8, 2020

Choose a reason for hiding this comment

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

Add a comment to look in build/libs

philburk reviewed Aug 21, 2020

View reviewed changes

DivineThreepwood approved these changes Sep 8, 2020

View reviewed changes

philburk closed this Oct 30, 2020

philburk mentioned this pull request Oct 30, 2020

Publish JSyn on Maven #82

Open

ligi mentioned this pull request Aug 25, 2021

maven central deployment #54

Closed

philburk mentioned this pull request Sep 2, 2021

Ant build Failing due to slf4j dependency #101

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Restructured project, added gradle, JUnit, logger, and more #83

Restructured project, added gradle, JUnit, logger, and more #83

RubbaBoy commented Jul 6, 2020

philburk commented Jul 6, 2020

RubbaBoy commented Jul 6, 2020

philburk left a comment

philburk Jul 6, 2020

RubbaBoy Jul 6, 2020

philburk Jul 6, 2020

RubbaBoy Jul 6, 2020

philburk Jul 6, 2020

philburk Aug 8, 2020

philburk Jul 6, 2020

DivineThreepwood Sep 8, 2020

philburk Oct 30, 2020

philburk Jul 6, 2020

RubbaBoy Jul 7, 2020

philburk Aug 8, 2020

RubbaBoy commented Jul 6, 2020

RubbaBoy commented Jul 7, 2020

philburk Aug 8, 2020

philburk Aug 8, 2020

philburk left a comment

philburk Aug 8, 2020

philburk left a comment •

edited

Loading

DivineThreepwood left a comment

philburk commented Oct 30, 2020

philburk commented Oct 30, 2020

		@@ -1,66 +0,0 @@
		<project name="JSynProject" default="dist" basedir=".">

Restructured project, added gradle, JUnit, logger, and more #83

Restructured project, added gradle, JUnit, logger, and more #83

Conversation

RubbaBoy commented Jul 6, 2020

philburk commented Jul 6, 2020

RubbaBoy commented Jul 6, 2020

philburk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RubbaBoy commented Jul 6, 2020

RubbaBoy commented Jul 7, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

philburk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

philburk left a comment • edited Loading

Choose a reason for hiding this comment

DivineThreepwood left a comment

Choose a reason for hiding this comment

philburk commented Oct 30, 2020

philburk commented Oct 30, 2020

philburk left a comment •

edited

Loading