search

MetroCS / redistricting

Experimentation with geopolitical redistricting
GNU Affero General Public License v3.0
5 stars 75 forks source link

Aleks branch #207

Closed Aleksander-Gomez closed 12 months ago

Aleksander-Gomez commented 1 year ago

Changes to contributing.md file

Aleksander-Gomez commented 12 months ago

I agree, this PR is not as inline with the acceptance criteria for #201. I intended to follow up my PR with an email regarding that, so I apologize for the confusing PR.

I wanted to inquire what would be the best solution for sticking with the content and language of my latest PR, whether I should change the acceptance criteria or apply it else where.

As you've suggested, perhaps this PR would be best as a comment under epic

80. A comment to request that Junit be listed as a tool, because building

requires Junit? Would this be a suitable comment provide basis?

On Sun, Dec 3, 2023 at 6:29 PM Jody Paul @.***> wrote:

@.**** requested changes on this pull request.

This isn't an obvious match with the acceptance criteria for #201 https://github.com/MetroCS/redistricting/issues/201

It may be that the information in this PR is actually most useful at this time as a comment to the issue (#201 https://github.com/MetroCS/redistricting/issues/201) or a comment to the original epic (#80 https://github.com/MetroCS/redistricting/issues/80). That would provide a basis for a well-considered modification to the product documentation.

The PR makes reference to a tool/technology (JUnit) not already represented in the document. The information in this PR needs significantly more context and vetting of the proposed explanation and solution. A section labeled "Junit.test.launcher" doesn't fit with the structure of the rest of the document.

This suggests that the document needs to be modified to include the tools/techs that are invoked via the build process (using ant and build.xml). Issues that arise with a particular tool would thus fit under that tool's heading/section.

FYI, below is the listing of primary targets of the build script as shown by ant. Looking at build.xml we can see that they invoke several additional tools/technologies that are used and expected to work as intended: Checkstyle, PMD, CPD, JaCoCo, JUnit, SpotBugs, ProGuard, Java2HTML

% ant -p Buildfile: redistricting/build.xml

  Build file for Redistricting project investigation

Main targets:

all clean, generate documentation, analyze source code, run unit tests checkstyle generate checkstyle report clean clean up dynamically-created files and directories coverage Run unit tests with JaCoCo instrumentation and format results cpd proccess source with CPD dist Use ProGuard to create distribution jar doc generate usage documentation doc-private generate maintenance documentation format generate formatted versions of source code info show build/release information jar Creates a jar file for the product pmd process source with PMD release Creates a jar file for the product release spotbugs process source files with SpotBugs test run unit tests Default target: all %

— Reply to this email directly, view it on GitHub https://github.com/MetroCS/redistricting/pull/207#pullrequestreview-1761466549, or unsubscribe https://github.com/notifications/unsubscribe-auth/AWZJ3AIALY47OWRKUAFS62DYHUROLAVCNFSM6AAAAABAB4YYL6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTONRRGQ3DMNJUHE . You are receiving this because you authored the thread.Message ID: @.***>

jody commented 12 months ago

My recommendation is to add your suggestions as comments to Issue #201 I'll just close this PR. (The title didn't add anything that needs to be preserved.)