serpent-tools icon indicating copy to clipboard operation
serpent-tools copied to clipboard

[BUG] Fix of MicroXSReader precheck mechanics

Open aalfonsi opened this issue 1 year ago • 9 comments

Please add the appropriate tag to the beginning of the title of your PR:

  1. [DOC] if documentation related
  2. [DEP] if something is deprecated or a deprecated feature is removed
  3. [ENH] if a feature or enhancement
  4. [BUG] if a bug fix
  5. [API] if a (possibly incompatible) change to the API
  6. [TST] if related to our test suite
  7. [STY] if related to code cleanliness and pythonicness

For bug fixes and enhancements, link to the corresponding issue.

Closes https://github.com/CORE-GATECH-GROUP/serpent-tools/issues/464

Make sure you have read over the developer guide to ease the review process. These include:

Include a thorough and concise overview of the changes, and why this PR is overall beneficial to the project.

Once the PR has been created and is nearing closure, make sure that serious changes, including deprecations and incompatible changes are documented in the changelog

aalfonsi avatar Aug 06 '22 18:08 aalfonsi

Thanks @aalfonsi @drewejohnson - should I take any action?

DanKotlyar avatar Sep 19 '22 08:09 DanKotlyar

@DanKotlyar I'm not sure why py 3.9 isn't going in testing. But once that gets resolved you should be able to merge this in

drewejohnson avatar Oct 14 '22 17:10 drewejohnson

@aalfonsi can you change the source branch from develop to master? We don't have py3.9 testing on develop -

https://github.com/CORE-GATECH-GROUP/serpent-tools/blob/f61cd104bd997aa80429f1059bbc3669adc18654/.github/workflows/testing.yaml#L20

drewejohnson avatar Oct 14 '22 17:10 drewejohnson

Unfortunately if I do, this PR will add ~90 files with several conflicts:

Screen Shot 2022-10-14 at 12 22 18 PM

Probably the best solution (if master is the real "default" branch) is for me to apply these changes in a new branch from master. What do you think?

aalfonsi avatar Oct 14 '22 18:10 aalfonsi

Unfortunately if I do, this PR will add ~90 files with several conflicts:

Screen Shot 2022-10-14 at 12 22 18 PM

Probably the best solution (if master is the real "default" branch) is for me to apply these changes in a new branch from master. What do you think?

Oh never mind...you said it :) "source branch" not "target branch". No problem...I can do it.

aalfonsi avatar Oct 14 '22 18:10 aalfonsi

microxs_patch.txt

Since the "master" branch is neither a protected branch or a default branch, it does not show up in my fork. The only way to contribute to master for me is to create a patch file (as attached).

aalfonsi avatar Oct 14 '22 19:10 aalfonsi

@drewejohnson How should I proceed with this?

DanKotlyar avatar Oct 15 '22 10:10 DanKotlyar

@drewejohnson How should I proceed with this?

Since the modification is quite small, Can one of you apply the patch in a new branch (based on master) and then we can close this MR ?

aalfonsi avatar Oct 17 '22 18:10 aalfonsi

@aalfonsi sorry for the confusion. I'll take your patch and create a new PR off master

@DanKotlyar can you take a look at these comments - https://github.com/CORE-GATECH-GROUP/serpent-tools/issues/466#issuecomment-1279286356 This should help resolve some confusion going forward

drewejohnson avatar Oct 17 '22 23:10 drewejohnson