Skip to content

Replace open structs with structs#6

Merged
steve-sullivan merged 1 commit intomainfrom
AP-584-Replace-OStructs
Mar 6, 2026
Merged

Replace open structs with structs#6
steve-sullivan merged 1 commit intomainfrom
AP-584-Replace-OStructs

Conversation

@steve-sullivan
Copy link
Contributor

No description provided.

Copy link
Member

@awilfox awilfox left a comment

Choose a reason for hiding this comment

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

Couple of questions, but overall looks like a great change.


# First load all UCPath Employee Data into obj
# rubocop:disable Metrics/AbcSize, Metrics/MethodLength, Metrics/PerceivedComplexity, Metrics/CyclomaticComplexity
#
Copy link
Member

Choose a reason for hiding this comment

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

Probably unnecessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll tweak that with my next code change which will be touching on the user.rb file

dbdef: "VARCHAR(16)"
status: OPTIONAL

- name: job_code
Copy link
Member

Choose a reason for hiding this comment

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

Is this a related change? Why are these fields being removed? (should this be a separate change?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had 2 duplicate codes (job_code and dept_code...guessing a copy/paste error ages ago). Structs are more strict than ostructs caught the issue.

@steve-sullivan steve-sullivan merged commit 5052753 into main Mar 6, 2026
5 checks passed
@steve-sullivan steve-sullivan deleted the AP-584-Replace-OStructs branch March 6, 2026 18:01
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.

3 participants