feature/#127-parking-api-mockup #174
No reviewers
Labels
No labels
_CRITICAL_
API
app
backEnd
Blocked, waiting for further changes
bug
cleanup
close
design
duplicate
enhancement
feature request
frontEnd
help wanted
invalid
low priority
needs input
needs review
project documentation
question
research
reviewed
script
security
SQL
style
testing
topLevel
wontfix
No milestone
No project
5 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
Andras/BoundlessFlowCampus2K!174
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feature/#127-parking-api-mockup"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Changes in this pr
The files that needs review :
The files that are part of dotnet project configuration that simply you can ignore:
The issues that can be closed after review and merge:
Assigned Hugo to review this issue.
After looking over the changes I don't see any issues that stands out. It follows the standards, is commented where needed and has a very detailed and easy-to-follow README file.
I switched to the branch on my local computer, followed the README instructions to test the new function, and it seems to work without any issues. I got this result from the test code that you insert in frontend/src/app/App.js:
@b24hugst
can you double check the request because the result you should get looks like this.
@a24sakma I redid everything but still get the same "null" result. Maybe someone else could just test the branch as well and if that person also gets your correct result we know it's probably just me 😂
@b24hugst Otherwise I can check while you build it because it should work on your pc.
After some communication help from @a24sakma, it now works as it should.
The problem was that I had some underlying cache of docker images or something that disrupted the results. After using docker system prune -a and then manually creating each docker file again, it worked correctly:
a24timsv has been assigned to make an additional review since the developer had configuration problems with docker.
I can confirm that it works, it just took some time due to other issues.
Since the parking mockup doesn't use any databases or similar it is assumed to be an external service (like TimeEdit) that is called upon rather than a local one (like the temp sensor). We could open an issue about designing how the data is shown once this gets pushed.
However, when making branches it should not be required to configure and build stuff manually to test a solution. As an example, this branch needs to manually build the image for the mockup data and add lines to App.js to showcase that it works. All branches should be runnable by pulling the branch and using the build scripts. We will get more errors by people manually configuring than not doing so. It is not needed now since we know that it works, but maybe this should be made aware to everyone.
@a24timsv
Nice that it works.
This I don't think I have the build script in my local machine at the momentI have the build script in my local machine. I just did not see it :). I will add it to the next pr that I am already working as part of the below changes
Added code to App.js and the build scripts to get it to work right after just running the build script.
This was manually merged. The review seems complete so i will close this pull request.
Pull request closed