Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Allow overriding response MSH content #103 #106

Conversation

jtarvainen
Copy link

@jtarvainen jtarvainen commented Sep 19, 2024

Issue

#103

Search terms

MSH, override

Questioner

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

Other things:

  • Write a unit test or update unit tests that cover your change in the code?
  • Set the PR to merge into the develop branch?
  • Clear documentation per the guidelines in the README.md as we are support medical applications?

@jtarvainen
Copy link
Author

jtarvainen commented Sep 19, 2024

I replaced the existing overrideMSH parameter (boolean), which was only used to set the MSH.9.3 value, with a comprehensive mshOverrides parameter (object) that allows overriding any MSH field value. This is a breaking change, but to me it makes sense to handle all possible MSH overrides with a single parameter.

So, to me, this change would bump the version to 3.0.0, but I didn't want to update the version on my own before discussing it here first

@Bugs5382
Copy link
Owner

@jtarvainen Hello! It's my normal Saturday to code and checking this out!

@@ -11,6 +11,7 @@ import {
HL7_2_5_1, HL7_2_6, HL7_2_7, HL7_2_7_1, HL7_2_8
} from 'node-hl7-client/hl7'
import { PROTOCOL_MLLP_FOOTER, PROTOCOL_MLLP_HEADER } from '../../utils/constants.js'
import type { ListenerOptions } from '../../utils/normalize'
Copy link
Owner

Choose a reason for hiding this comment

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

@jtarvainen just needs to change to .js, but I will do that in the backend during merge

@Bugs5382 Bugs5382 changed the base branch from develop to 103-feat-allow-overriding-response-msh-content September 21, 2024 16:17
@Bugs5382 Bugs5382 merged commit 1dcef70 into Bugs5382:103-feat-allow-overriding-response-msh-content Sep 21, 2024
1 check passed
Bugs5382 pushed a commit that referenced this pull request Sep 21, 2024
# [3.0.0-beta.1](v2.5.0-beta.1...v3.0.0-beta.1) (2024-09-21)

* 103 feat allow overriding response msh content ([#107](#107)) ([ba7ec92](ba7ec92))

### Features

* Allow overriding response MSH content [#103](#103) ([b208429](b208429))
* Allow overriding response MSH content [#103](#103) ([#106](#106)) ([1dcef70](1dcef70))

### BREAKING CHANGES

* MSH Override Changes
* MSH Override Changes
@Bugs5382
Copy link
Owner

🎉 This PR is included in version 3.0.0-beta.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Bugs5382 pushed a commit that referenced this pull request Sep 21, 2024
# [3.0.0](v2.5.0...v3.0.0) (2024-09-21)

* 103 feat allow overriding response msh content ([#107](#107)) ([ba7ec92](ba7ec92))

### Features

* 3.0.0 ([#108](#108)) ([1252e23](1252e23))
* Allow overriding response MSH content [#103](#103) ([b208429](b208429))
* Allow overriding response MSH content [#103](#103) ([#106](#106)) ([1dcef70](1dcef70))

### BREAKING CHANGES

* MSH Override Changes
* MSH Override Changes
@Bugs5382
Copy link
Owner

🎉 This PR is included in version 3.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants