ucsb-cs56-projects / cs56-music-basic-synthesis-demo

-
0 stars 5 forks source link

Clean up of documentation, build.xml, repo #18

Closed pconrad closed 10 years ago

pconrad commented 11 years ago

This project has had several programmers working on it, and as such there is a lot of cleanup that needs to happen.

The build.xml has lots of commented out stuff that doesn't make sense, and the targets are not really ones that make sense.

Also, the only target that has a description on it is one that should be eliminated---the "linenumbers" target.

The html subdirectory should be gotten rid of---its from the old way of doing packages.

Make sure that the README and the build.xml have adequate documentation and appropriate targets to run ALL of the code. Look at some other examples of projects that use -Darg logic to get command line parameters from the user and pass them into the target inside the build.xml file, and modify the build.xml here apporpriately.

And edit the README.md so it has a roadmap for the entire repos software.

~estimated 60

pconrad commented 10 years ago

@rtwaltman and @dwang68

rtwaltman commented 10 years ago

@bronhuston I've completely cleaned build.xml to remove redundancies and unused portions. General usability and readability has been vastly improved. Descriptions have been added for all targets. Command line running for melodyGUI, melody, and basicGUI has been implemented. Most targets have been rewritten for clarity, using Professor Conrad's build.xml files from labs as a reference. Javadoc URL had "13S" present in it, which I changed to "W14."

html subdirectory has been deleted.

README.md has been mildly updated to reflect changes to build.xml for command line running of various classpaths (-Darg).

The code was given to us in a broken state (meaning gui buttons are broken and no sound is ever output), so until I fix it, is it okay if I wait to fully update the README? UPDATE: The code works fine from CSIL and campus machines and the errors were all likely due to putty, as I had only worked on it at home until now.

Thank you! :)

bronhuston commented 10 years ago

@rtwaltman First, you should push all of the changes you made into your version of the directory that you forked.

Second, when I test the current code, the GUI seems to be working, soI think you might be running the wrong version of the GUI. Try running the gui with the command java -cp build edu.ucsb.cs56.projects.music.basic_synthesis_demo.MelodyGui and you should see that the the buttons seem to be working and sounds are produced.

As for the pull request, you should probably wait until you have finished the other issues for project 1 before doing the request. For project 1, you have to accumulate 500 points, and this issue(#18) is only estimated to 60 points, so you still need 440 more points. These points should be gotten from the other current issues posted #19, #20, etc.

After you have finished enough of the issues to have at least 500 points, you would then follow the steps posted on the wiki to do the pull request.

Hopefully this answers you're questions, but if not send me an email or talk to me in lab later today!

rtwaltman commented 10 years ago

@bronhuston Thanks for the response! Professor Conrad indicated that it is best to work out of the same fork, as it would be difficult to keep both Dalin (@dwang68) and I synced up otherwise. So I am a collaborator on his fork and we're both working out of that right now, which I'm sure is what you meant - just thought I would clarify just in case.

Also, I edited the first comment above, but probably not before you responded. I found out putty was causing all the issues I was having with the GUI.

As for the points, thank you, I am definitely aware that I have a lot of work ahead of me for the rest of today and tomorrow haha. Here we go! :)

Also, today in class Professor Conrad told us to pull request when we have finished an issue. I think the main reason for that is so that the class is able to do peer code reviews on each others' pull requests for additional points.

bronhuston commented 10 years ago

@rtwaltman Oh oops, I forgot to check his fork of the repo to see the changes before! The changes look good so far so just ignore the first part of my response before!

I'm glad you figured out that putty was causing the problems, and that its all fixed!

I didn't realize Professor Conrad said to do a pull request after each issue, so just disregard what I said before and go ahead and do a pull request for this issue.

Sorry for any confusion about the pull request that my response might have caused, and let me know if you have any more questions!

rtwaltman commented 10 years ago

@bronhuston I just issued the pull request. First to your master (oops!) and then to the correct (at least I believe) master of the UCSB level master.

Links: Wrong pull request to bronhuston repo (doesn't look like I can delete it myself): https://github.com/bronhuston/cs56-music-basic-synthesis-demo/pull/1 Correct pull request: https://github.com/UCSB-CS56-Projects/cs56-music-basic-synthesis-demo/pull/23

rtwaltman commented 10 years ago

@bronhuston I believe I've just realized something. I don't believe -Darg logic was fully implemented. While the "ant basicGUI" and "ant melodyGUI" are working correctly, the "ant melody" only sends the basic

"java -cp build edu.ucsb.cs56.projects.music.basic_synthesis_demo.Melody_Code.ADSREnvelopedContinuousSound"

as the command. I need to implement a way for the 7 arguments (frequency, amp, attack, decay, sustain amp, sustain time, and release) to be set by the user in command line after "ant melody"

Also, I will implement ant methods to run the .txt files as well, as those are currently unsupported via build.xml.

rtwaltman commented 10 years ago

@bronhuston Sorry for all the updates! I know you have other student/obligations, so don't feel pressured to focus entirely on me. I really just have one more question on this.

I've figured -Darg out and got it working, although with 7 arguments, typing out -Darg0 through -Darg6 is hardly any better than writing out the full classpath, so I think it might be wise to just have an echo that we can copy & paste. It would actually be faster and easier than writing 6 -Darg's for the user. I could do this in "ant run" for each of the options. i.e. it echos similarly to how it does currently but with all the -Darg written out already to be filled in a little easier for running the ADSREnvelopedContinuousSound.

With that said, here is my question: Should I make "ant melody" run the default.txt melody and then make a seperate "ant ADSR" to run ADSREnvelopedContinuousSound? The code that was given to us had defined running 'melody' as the ADSR, but I could easily differentiate them.