Skip to content

Conversation

@SoggyRhino
Copy link
Contributor

parser, sectionParser, courseParser

  • cleaned some of up the code I added in my last PR

parser_test.go

  • moved testHelper.go into parser_test.go
  • moved the testdata loading code TestMain so tests can run in parallel
  • made the test data struct include more fields
  • added flag to regenerate /testdata
    • go test -v ./parser -args -update

other_tests

  • minor changes, basically the same

I noticed there is an issue for writing go docs so I wrote some for the stuff I was working on. I had more problems with grade loading so once this is done I'm probably going to work on that.

Copy link
Contributor

@jpahm jpahm left a comment

Choose a reason for hiding this comment

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

Looks pretty solid, just some small changes to request!

@SoggyRhino
Copy link
Contributor Author

True, I didn't consider that some we actually need use some of the fields in the parser. I also added panics to getPrefixAndNumber and getInternalClassAndCourseNum since we depend on them to do map access. Test have been updated to expect panics on bad inputs.

I clicked resolve for all the issues. Not sure if I'm supposed to do that but they said outdated so I assumed if there was a problem any of the changes you'd make a new one with the updated code.

@jpahm
Copy link
Contributor

jpahm commented Mar 11, 2025

You did it properly, thank you!

Copy link
Contributor

@jpahm jpahm left a comment

Choose a reason for hiding this comment

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

Looks good now!

@jpahm jpahm merged commit 0d8cdf9 into UTDNebula:develop Mar 11, 2025
2 checks passed
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