Merge Bug Fixes #1048
Merge Bug Fixes #1048
Conversation
This commit sets the default precision of part parameter searching to femto to maintain fractional values.
This commit adds the part price information to the stock history. The average price of a part doesn't change when stock is depleted
This commit sets the default category to null upon part creation. This is in effort to make the user pick the correct category.
This commit fixes the exporter adding accented characters to the file.
This commit keeps the project remark connected to the newly selected items in a project report.
* populate all form-items with values from selected parameter-record `onParameterSelect(...)` * normalize order of properties (list unit name after id) Signed-off-by: Florian Kerle <[email protected]>
This commit closes issue partkeepr#418. Double clicking on a part-item in a 'Storage Location' opens the part details window.
Do not rely on deprecated the URL.createObjectURL for MediaStream, other than as a fallback for old browsers. Firefox (62+) have deprecated it, Safari have deprecated it, and Chrome have started the process to deprecate this as well. I should note that I have not built this code myself and tested it, but the issue partkeepr#1002 is still present in the demo site and this code here is simply adopted from the developer site of Mozilla. Sources: https://bugs.webkit.org/show_bug.cgi?id=167518 https://bugs.chromium.org/p/chromium/issues/detail?id=800767#c28 https://www.fxsitecompat.com/en-CA/docs/2018/url-createobjecturl-no-longer-accepts-mediastream-as-argument/ https://developer.mozilla.org/sv-SE/docs/Web/API/HTMLMediaElement/srcObject#Supporting_fallback_to_the_src_property
|
hey @vmw I'm going to merge my ci/cd fixes soon (want to make sure I don't change anything important) and then lets see how your branch builds against develop |
|
Although it would've been easier to review if the bugs where treated in their own PR |
|
Ok yeah you're going to need to break this up in to more manageable chunks. I saw a couple of very useful fixes, but each really deserves their own PR that can individually be reviewed and merged. |
|
@vmw Vincent - could you create individual PRs? it will enable the merge far easier and also allow us to note which bug each one fixes... sorry for the inconvenience, but sometimes merging PR can be really hard.. |
|
Yes: I'll look into breaking them up to make it easier. Unfortunately, we forked this build, and, as you previously noted, split up some of the commits to make it easier for ourselves (internally). I'll see whether I can cherry-pick some from master and I'll list them as separate requests. I fully expect that some of them will (should?) be rejected as we opted to deliberated work around some areas of the code that were causing us issues. |
|
This might take a bit of time because, for some of the commits, we included temporary files which should be cleaned up. I'm looking into this now. |
|
Cool, looking forward to see some of the bug-fixes. I want to get this one travis-ci bug fixed though, before we start merging new code. |
|
I've created separate bug fixes, but it's going to fail the style guide. I had previously fixed that manually, but you if you accept the previous PRs, you should be able to easily use this commit to clean up the code, as it's all in a single commit (the last two). |
|
Thnx for opening these. Best would be to rebase your code to the current master to pull in all configuration and style changes, so we can see the working builds coming in (lets fix travis-ci first though). |
|
@dromer I realize the style CI should have been implemented within each commit. Unfortunately, we don't immediately have the engineering resources to review each commit - I did try and break up each commit as best I could manually, but I don't think I can polish each one to the level they should be. The problem is that we do development locally, so we don't realize the style guide issues until after we push the fixes. At the time this was done, we didn't realize it was a problem until the last minute, so I'm the one who came in to fix it one-by-one. |
Yes I know, that's what I don't like about styleci, there's no way to easily run it locally for review (we use flake8 at work saves a lot of headaches). That's why you could merge master branch back into your pr-branches and pull in the fixes that way. |
|
Ok, current master should have working builds (style and unit tests) again. Please if you can do a |
|
I don't immediately have the bandwidth to do this - it will likely take me a several weeks to get to this. We're using our forked version, so I'm happy to close the PRs if you'd prefer that instead and manage this downstream for the time being? |
|
I'm not sure what you mean. What should we manage? |
|
Sorry, I was unclear: We don't mind hosting a downstream version of partkeepr and simply cherry-picking or patching the upstream version. The problem is that we don't immediately have the bandwidth to do the complete code review - the engineer working on this isn't here anymore. As a result, it falls on me, and I don't have the bandwidth to properly format the code. We know it works, and compiles, on our current installation, and this (mega) commit also had the style guide fixes applied on top of all the bug fixes. I definitely realize that this is far from the best PR, and also note that the bug fixes aren't atomic. I did try and clean that up, at least, in the individual PRs. However, because the entire style guide review process requires the git website, it's a lot more work to go back and individually fix the separate PRs for style. (This was actually the most time consuming part of the process - if you have a command line linter or something I could run locally - similar to flake8 or lint - it would speed things up). I definitely understand that this is undesirable grunt work, so I'm just suggesting that I close them and I maintain them locally until we have the bandwidth to fix them. |
|
I agree that it is quite annoying that you can't run StyleCI locally, but this is what we have at the moment. |
|
Would something like this not work for styling php in PartKeepr?
https://mike42.me/blog/2018-06-how-to-check-php-code-style-in-a-continuous-integration-environment
John Pateman
…Sent from my iPhone
On 19 Feb 2020, at 07:31, dromer ***@***.***> wrote:
I agree that it is quite annoying that you can't run StyleCI locally, but this is what we have at the moment.
(I don't know a decent equivalent for PHP like flake8)
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
|
I would consider PHP_CodeSniffer a useful addition. @Gasman2014 did you use it yet? Is StyleCI configured to be running on any of the provided styles that PHP_CodeSniffer provides? Adding it to travis should be no big problem. |
|
I have no experience of using this (I mainly use python / c ) but, if it does what it says on the tin, it might go some way towards mitigating your code formatting issues.
John Pateman
…Sent from my iPhone
On 19 Feb 2020, at 09:46, Christian ***@***.***> wrote:
I would consider PHP_CodeSniffer a useful addition. @Gasman2014 did you use it yet? Is StyleCI configured to be running on any of the provided styles that PHP_CodeSniffer provides?
Adding it to travis should be no big problem.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
|
@vmw @Gasman2014 Please have a look at #1092. |
|
Is it possible to re-run the travis-ci tests? I recall that there may have been issues with the CI system, and I'm interested in seeing whether it now passes all of them. |
|
I've also updated the conflicts against this branch, if you'd also like to take a look at this one. |
|
It would be much better if each of these fixes come in as separate PRs. Further more I see a lot of small issues with this PR. Random things commented out or unused things brought in. As is I don't see why this would be merged. |
This branch contains the following bug fixes;
fixes: #997, #989, #418, #1002
It also fixes exporting issues, allows the root category to not be set as default, fixes prices in stoc history, and fixes a rounding issues in parameter searching.