chstudebaker / herobase

0 stars 0 forks source link

Week05 ready for reveiw #5

Closed chstudebaker closed 7 months ago

chstudebaker commented 8 months ago

@pawaitemadisoncollege week 05 ready for review

What were your key learning points/takeaways? How to get the database set up so it could add both a hero and a power at the same time, as well as delete them at the same time What challenged you? Because this idea was so challenging to wrap my head around, I consider it a valuable learning experience What problems did you solve and what resources did you use to solve them? AI helped, but most of it was trial and error. The database structure changed multiple times over the course of the project, and it took me at least five tries to find one that suited what I was trying to do.

pawaitemadisoncollege commented 8 months ago

Hi @chstudebaker - your week 5 work has the same issues I mentioned in https://github.com/chstudebaker/herobase/issues/4.

In addition:

Once you have made the revisions suggested above and in the week 4 issue, please respond to this issue and I'll review.

chstudebaker commented 8 months ago

@pawaitemadisoncollege Week05 updates completed. Ready for re-review

pawaitemadisoncollege commented 8 months ago

Hi @chstudebaker - More good work here! Refer to https://github.com/chstudebaker/herobase/issues/3. Resolve those first, and then continuing with this issue. The same comments from that issue apply to the power dao. In addition:

Let me know when this is ready for another look.

chstudebaker commented 8 months ago

@pawaitemadisoncollege Added full testing screenshot + fixed week 04. Hope this is what you were looking for

pawaitemadisoncollege commented 8 months ago

I would expect to see all unit tests run and showing 100% of method coverages, but I don't see that. Can you run all tests at the same time and show coverage. If it's not working for you for some reason, record a video showing what you are doing and the result and post in #help.

I also noticed some println statements. Remember the best practice is to use a Log4j. https://github.com/chstudebaker/herobase/blob/dedfa06241111f41cd115fe422f6c32534b3b769/src/test/java/persistance/powerDaoTest.txt#L136

chstudebaker commented 8 months ago

@pawaitemadisoncollege Fixed this. removed and replaced println's and put my run all tests at once screenie in the screenshots folder

pawaitemadisoncollege commented 8 months ago

Week 5 screenshot showing all tests running look good!

I did notice somesnreaky printlns in your project yet. Screenshot 2024-03-27 at 12 04 59 PM