search

PollyNET / Pollynet_Processing_Chain

NRT lidar data processing program for multiwavelength polarization Raman lidar network (PollyNET)
https://polly.tropos.de/
GNU General Public License v3.0
20 stars 8 forks source link

Dev #154

Closed HolgerPollyNet closed 2 years ago

HolgerPollyNet commented 2 years ago

Can be merged already? WOuld be perfect for the re-processing of sime data...

ZPYin commented 2 years ago

Looks good enough.

Btw, I have added you to the Admin list of this repository. Now you would have the full right to merge PRs. I just noticed this...😄

HolgerPollyNet commented 2 years ago

@ulysses78 Can you make abrief review then I could merge....

ulysses78 commented 2 years ago

Yes I will have a look at this.

ulysses78 commented 2 years ago

One question arises... there was a pull from Holger to master-branch (on 2022-04-11), which is not yet in the dev-branch: https://github.com/PollyNET/Pollynet_Processing_Chain/pull/151. Hopefully those changes are not in the same file positions as in the pull request #154 !?! If this is no problem, #154 can be pulled + merged.

In general I suggest working with more flexibility in terms of using "better" wildcards and/or regular expressions (e.g. case insensitivity or '[0-9]' or '\d*' or '[a-z]', ...) in linked filenames. This would prevent those commits like in https://github.com/PollyNET/Pollynet_Processing_Chain/pull/154/commits/52843188398c3494521d8e5368ebb3a13da175c7 or in https://github.com/PollyNET/Pollynet_Processing_Chain/pull/154/commits/1a08fa6eb7778288d497a5a9c068a800d34adb77.

HolgerPollyNet commented 2 years ago

Okay, can we merge #151 and #154 during the pull request? #151 was a bug fix only relaized while working on the server... and the have synchronized master and dev branch? The we have a good basis for further developments within dev branch.

ulysses78 commented 2 years ago

We can just give it a try, I would say. @ZPYin , what do you say?

HolgerPollyNet commented 2 years ago

@ulysses78 You still need to make a review: grafik

ulysses78 commented 2 years ago

Interestingly there are still some non-unix characters '^M' in lib/io/loadMeteor.m. But only in the commented out lines 108 and 131. So this seems to be no problem.

ZPYin commented 2 years ago

We can just give it a try, I would say. @ZPYin , what do you say?

I'm not sure. I don't have such experience.

But it's definitely possible to merge pull request with some file conflicts, see here. This is a common case for team collaborations, and can be resolved easily (either locally or through github portal). No worry for that.

ZPYin commented 2 years ago

... In general I suggest working with more flexibility in terms of using "better" wildcards and/or regular expressions (e.g. case insensitivity or '[0-9]' or '\d*' or '[a-z]', ...) in linked filenames. This would prevent those commits like in 5284318 or in 1a08fa6.

Agree. 👍