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

Section 8 update #92

Merged
merged 9 commits into from
Apr 12, 2023
Merged

Section 8 update #92

merged 9 commits into from
Apr 12, 2023

Conversation

Pelayo-Reguera
Copy link
Contributor

Update of the crosscutting concepts of the documentation

@Pelayo-Reguera Pelayo-Reguera added the documentation Improvements or additions to documentation label Apr 10, 2023
@Pelayo-Reguera Pelayo-Reguera self-assigned this Apr 10, 2023
@Pelayo-Reguera Pelayo-Reguera linked an issue Apr 10, 2023 that may be closed by this pull request
Copy link
Contributor

@GuillermoDylan GuillermoDylan left a comment

Choose a reason for hiding this comment

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

In the new Domain Model there are some <<"use">> that seem to be misplaced, there are also some "comment blocks" that are in Spanish, I don't really get why you are doing this and it may need to be discussed by the team if we are to leave it that way. And to conclude, you have also re-formatted some code from different classes, I don't know if something has been changed, as some imports have been removed, so please check that this does not interfere with the applications functionality and that everything works as intended.

@Pelayo-Reguera
Copy link
Contributor Author

In the new Domain Model there are some <<"use">> that seem to be misplaced, there are also some "comment blocks" that are in Spanish, I don't really get why you are doing this and it may need to be discussed by the team if we are to leave it that way. And to conclude, you have also re-formatted some code from different classes, I don't know if something has been changed, as some imports have been removed, so please check that this does not interfere with the applications functionality and that everything works as intended.

Obviously I will NOT leave it as it is. That is why is a DRAFT pull request and not a normal one. The imports deleted are colored in gray when they are not used in the code so it shouldn't affect the application. And the comments are there to show that I didn't find or whatever and I want help or your opinion about that.

Copy link
Contributor

@uo282892 uo282892 left a comment

Choose a reason for hiding this comment

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

About the questions in the image:

  • The api is supposed to be used by the map (to retrieve places).
  • Category is a field of the places.
  • PlaceType is not a class, it has to be either a type or an interface extending Document to work with mongoose (MongoDB).

After solving that, it would be nice, if possible, to change a bit the background color of the folders to see them better (the lines of the folders mix with those of the classes and associations and it is a bit difficult to distinguish everything.

Finally, although I understand this is not needed for now (as it would take some time), I think it is necessary to rearrange everything to minimize the number of crossed lines and position the items as tidy as possible.

Once you solve the initial remarks, it would be ok for me in this delivery.

@Pelayo-Reguera
Copy link
Contributor Author

@Davidmc07 which was the purpose of having a folder called place in the user POD? Are there the public places or what?

@Pelayo-Reguera Pelayo-Reguera marked this pull request as ready for review April 12, 2023 21:03
# Conflicts:
#	webapp/src/adapters/solid/Assembler.ts
#	webapp/src/adapters/solid/PODManager.ts
@Davidmc07
Copy link
Contributor

@Davidmc07 which was the purpose of having a folder called place in the user POD? Are there the public places or what?

The places folder contains the places created by the user. Their privacy level can be changed individually.

@Pelayo-Reguera Pelayo-Reguera merged commit fae297e into develop Apr 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Section 8 - Cross cutting concepts
5 participants