Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Activity
  • May 13 18:27
    knjk04 commented #696
  • May 13 18:26
    knjk04 commented #696
  • May 13 15:35
    jwlai commented #696
  • May 13 13:15
    jwlai commented #696
  • May 13 13:15
    jwlai commented #696
  • May 13 13:12
    jwlai commented #696
  • May 13 13:12
    jwlai commented #696
  • May 13 13:03
    jwlai commented #696
  • May 13 11:54
    knjk04 synchronize #636
  • May 13 06:32
    knjk04 synchronize #636
  • May 13 05:36
    knjk04 closed #226
  • May 13 05:36
    knjk04 commented #226
  • May 13 05:35
    knjk04 labeled #226
  • May 13 05:34
    knjk04 closed #667
  • May 13 05:34
    knjk04 closed #619
  • May 13 05:34
    knjk04 closed #590
  • May 13 05:34
    knjk04 commented #590
  • May 13 05:34
    knjk04 closed #701
  • May 13 05:34
    knjk04 commented #701
  • May 13 05:33
    knjk04 labeled #701
Karan Kumar
@knjk04
OK, I'll change the scope of the issue
Hopefully, that will make it easier as well
Joseph Kenny
@josephkenny
We could just use one of the existing filters and make it an all-in-one.
Do you think we should keep the design how it is now with a filter component? Since all we have is a table. It's not like we're pulling results after entering a search query.
5 replies
inhundreds
@inhundreds

Hi All, for issue #215 I'm adding test to the StatisticsView constructor but I'm having trouble with this code:

    public StatisticsView(PredefinedShelfService predefinedShelfService) {
        RatingStatistics ratingStatistics = new RatingStatistics(predefinedShelfService);
        GenreStatistics genreStatistics = new GenreStatistics(predefinedShelfService);
        PageStatistics pageStatistics = new PageStatistics(predefinedShelfService);

        // rest of the constructor

    }

How can I mock the 3 Statistics objects? I want to change their behaviour in order to test the logic that build the view.

I moved the rest of the constructor in another method, passed the three objects as parameters and called the new method from the constructor. Can you thing of any cleaner solution?

I don't like my solution because when I want to test the method I have to reset the view already built calling the constructor.

Thanks,
Diego

Karan Kumar
@knjk04

Hi Diego,

What are you testing?

4 replies
Tom
@Tom-private
Hi everyone,
I have a question regarding you policy for conversations on a pull request.
Am I allowed to close a conversation when I made the changes and it was so minor, that no further comment was needed or should only the reviewer close them?
Kind regards,
Tom
Karan Kumar
@knjk04
@Tom-private It would be better to let the reviewer close them just so that they can better track whether the changes were made. It can be confusing if they're resolving conversations and a contributor is
2 replies
MikeLowrie
@MikeLowrie
Hi folks, I had two points regarding setup that I could really use a hand with:
1) Using IntelliJ 2017.3.4 for development and clicking Run on the specified Java file in step 4 does not cause any action in the IDE. Can anyone assist with this?
2) Going to localhost:8080 in Step 5 throws a 404 error. Not sure if this is due to a Docker issue I didn't see or because IntelliJ isn't running the code.
1 reply
For my second point, worth noting that localhost:8081 allows me to log in.
Karan Kumar
@knjk04

Hi @MikeLowrie,

1) Have you opened the run window? If nothing happens at all, as you say, then perhaps try restarting IntelliJ? From what you've told us, it seems that the app is running because you're able to access it (albeit on a different port). If that doesn't work, let us know and we can help you further

2) I don't know why what it is just yet. It's strange that you're able to access the app on port 8081, but not on 8080. I'll have a look to see whether I can also access the app on 8081.

3 replies
Karan Kumar
@knjk04
If you're reviewing code (or if you're just interested), we have a code reviews document that tries to standardise the way we review code
woodyinho
@woodyinho
Hi
I want to ask about how did you handle this, If I want to add a particular book to a custom shelf say book A to history shelf?
Karan Kumar
@knjk04

Hi,

Sorry, I don't understand your question. It's not clear to me whether you would like a high-level overview or whether you would like to understand how it works in code.

High-level overview:
It's currently a one-to-many relationship (one shelf can have many books) -- this will be changed soon to many-to-many. A user has to first create a custom shelf (e.g. a shelf called history). Then, a user can either create a new book (called book A, for example) or edit an existing book to store the custom shelf.

I hope this helps. If not, please could you clarify what specifically you would like to know and I, or someone else, would be happy to answer

woodyinho
@woodyinho
Oh thank you for clarifying and I meant how it works in code?
Karan Kumar
@knjk04

Relevant UI classes: BooksInShelfView and BookForm (which has recently been split)
Relevant entity classes Book, Author and PredefinedShelf (at the minimum). You can also look at their respective service and repository classes.

The BooksInShelfView represents the 'my books' page. Look out for the 'add book' button and what happens when pressed (it opens the 'BookForm' dialog).

Next, look in the 'BookForm' for where we use 'Shelf'. You'll find it in places like the method where we bind it for things like validation. Also, look out for where we have the form field.

I hope this points you in the right direction. If you have any specific questions, let us know

woodyinho
@woodyinho
Ahh that makes sense now
Thanks a lot, I really appreciate it
Karan Kumar
@knjk04
You're welcome
Karan Kumar
@knjk04
For anyone currently working on an issue, please note that we have updated the setup instructions, so please refer to the latest version of the README on the master branch
3 replies
Karan Kumar
@knjk04
If anyone is interested, issue #294 involves suggesting (but not implementing) ideas on where we could improve our test coverage
Tom
@Tom-private
Should PRs continuously be merged with the master? Or should this be avoided in order to avoid the risk of confusing the reviewers because the changes may get a bit incomprehensible?
1 reply
merged like rebased (but with "git merge master feature-branch") onto master not like "merge into master".
Tom
@Tom-private
Is anybody else unable to connect to the db with phpmyadmin? I had to set
environment:
PMA_HOST: "mysql" for the phpmyadmin-container in the docker-compose.yml in order to be able to connect.
3 replies
Tom
@Tom-private

Very important note to self and all other people that may work on the frontend:
(We should definitely add this somewhere, but I don't know where.)

When the app was build with mvn clean package -Pproduction (-> target folder was generated) and you change something in the css, nothing will happen!
You have to delete the target folder.
When the app is started by pressing "play" in intelliJ, a new target folder will be generated. You do not have to delete this one to be able to see changes to the frontend, just restart the app.

I have no idea why this is and what the difference between the target-folders is, this is just my finding. Maybe someone knows the cause for this.

1 reply
Karan Kumar
@knjk04
After our first release, we're looking to move away from Vaadin to Angular. If anyone wants us to use a different popular JS framework (e.g. React or Vue), then let us know here why they think that framework may be better
Karan Kumar
@knjk04

Anyone that has or is currently working on Vaadin-related issues: thank you for all of your help! It's definitely not wasted work. Whichever framework we go for, we will migrate with keeping all remaining functionality in mind. Vaadin has helped us get to where we are now very quickly, so it was useful to serve as a template of what we want to achieve in another framework.

Switching to another framework has been on the roadmap for some time. Before it was a case of if, now it's a case of when.

On an unrelated note, I'm conscious of the PR backlog. I've been busy throughout the week, but I am now working my way through them. Any existing pull requests should receive a review today
woodyinho
@woodyinho
How is the relationship works between a user and a shelf in general? and does this relation differs from custom shelf to predefined shelf?
16 replies
Karan Kumar
@knjk04

@Tom-private I'm looking at #310 now. Are you running this with step 1 or 2 (presuming that you're still Docker)? Also, what OS are you on?

The build fails for me with step 1 on Windows (not related to the issue you're on). I commented out the failing tests, but I was just curious to know whether you're also getting the same problem

3 replies
Tom
@Tom-private
Hi everyone,
sadly, I have to tell you that I will no longer have time to contribute on this project.
I have a very important exam and a final project coming up and need to prepare for that. Nevertheless, I will take care of my open PR until it is merged.
Working in this project was a lot of fun. This project is very organized which enabled me to easily start contributing. I will definitely recommend this project to everyone that is interested in contributing to an open source project.
I am exited to see, how this project evolves and wish you all success and good luck! :)
Kind regards, Tom
Karan Kumar
@knjk04

Hi Tom,

Thank you for your kind words & thank you for all of your help. I know you helped on a number of issues and that your work was of a consistently high standard. It's all very much appreciated!

Best of luck with your exam!

Christoph Flick
@ChFlick

After our first release, we're looking to move away from Vaadin to Angular. If anyone wants us to use a different popular JS framework (e.g. React or Vue), then let us know here why they think that framework may be better

I support the switch to a JS framework, I personally haven't really felt comfortable with Vaadin.
I like React and Vue more than Angular, but at the end of the day the framework doesn't really matter imo.
Do you have any plans on how to tackle the switch @knjk04 ?

Karan Kumar
@knjk04

@ChFlick Out of interest, why do you prefer React & Vue more than Angular?

After we finish with our MVP for 0.0.1 (and go live), I was thinking we start working on the migration (0.1.0 ). For the migration, I was thinking we should start building the app again by deleting all of the Vaadin related code and starting afresh. By pushing to a release branch, we can keep the master branch as a reference and try to replicate what we had before.

I contemplated trying to keep Vaadin while building Angular (or what other framework we end up using) and then slowly phase out Vaadin, but I think that's more effort than it's worth.

I don't know if that's the best way to go about this, so I'm open to any suggestions/alternative approaches

Karan Kumar
@knjk04

I'm also been thinking about transferring our repository from my personal account to an organisation to better reflect that this is a joint effort. If we create any other repositories (such as a GitHub pages repository for documentation), they can all fall under the same umbrella.

As far as I'm aware from the GitHub docs and having spoken to the GitHub customer support team, nothing should be lost in the transfer. If anyone has any concerns, let us know

Amit Garg
@amit1307
Just to give my input on the UI change, I feel this is a very good idea and we should do it sooner than later. I would just like to highlight couple of points that we can keep in mind while moving to JS:
  1. More emphasis of unit tests, we can advocate TDD if that doesn't sound too extreme.
  2. Smaller classes focussing on very small bits can help a lot as compared to the current case like BookForm is slowly becoming a God class.
Karan Kumar
@knjk04

@amit1307 I agree with all of this. As soon as our first prototype is out, we'll stop working with Vaadin entirely.

More emphasis of unit tests, we can advocate TDD if that doesn't sound too extreme.

This is a great point. At some point, we asked that all features and bug fixes come with accompanying unit tests, but I think there's more we can do to enforce that. TDD is great, although I'm not sure how we could enforce it. However, we could definitely advocate it and some of us could lead the way (personally, I haven't been doing this, but I should).

For our current tests that use Vaadin, we can also work on migrating that to whatever JS testing framework we go for.

It would have helped if we started writing unit tests from the start. We can do that this time and also better utilise codecov to see when a PR will decrease our code coverage

Smaller classes focussing on very small bits can help a lot as compared to the current case like BookForm is slowly becoming a God class.

Agreed. With the BookForm, that got out of hand too quickly (someone is working on refactoring this: see #249). One thing I think we could do better this time around is to refactor more as we go along. More people reviewing code can also help improve our code style.

These were all great points, so feel free to keep the suggestions coming.

Christoph Flick
@ChFlick
Thanks gitter... I just posted basically a wall of text and gitter just deleted it... I'll try again :D
1 reply
Christoph Flick
@ChFlick

@ChFlick Out of interest, why do you prefer React & Vue more than Angular?

In short:

  • gut feeling & personal experience of Angular being quite heavy/clunky/bloated
  • I like the "component first" approach of React & Vue, in Angular they feel a little bit like second class citizens

More emphasis of unit tests, we can advocate TDD if that doesn't sound too extreme.

I second that, unit testing should be more important and while we probably cannot enforce TDD, advocating it is a good idea.

Smaller classes focussing on very small bits can help a lot as compared to the current case like BookForm is slowly becoming a God class.

Super important and we should definitely focus this. Yet, I think Vaadin and the "Frontend in Java" approach is the main culprit here.

I'm also been thinking about transferring our repository from my personal account to an organisation to better reflect that this is a joint effort. If we create any other repositories (such as a GitHub pages repository for documentation), they can all fall under the same umbrella.

Good idea. Imo this will help building a community. Maybe we could switch to slack at the same time?

Karan Kumar
@knjk04
I haven't used any of the JS frameworks before (I'm currently learning Angular), so I'll take a look into React & Vue when I get some more time. I'll then come up with a short comparison with the pros & cons for each and then share it here to get feedback
Karan Kumar
@knjk04

I don't know if you saw, but I suggested switching to Slack earlier (I'm not so fond of Gitter). Discord was also suggested. Discord's nice, but not being able to start threads is, in my opinion, a deal-breaker.

Is everyone else happy to switch to Slack? Or if there are any other alternatives or arguments for Discord, please feel free to share

Christoph Flick
@ChFlick

I don't know if you saw, but I suggested switching to Slack earlier (I'm not so fond of Gitter). Discord was also suggested. Discord's nice, but not being able to start threads is, in my opinion, a deal-breaker.

Yes, that's the reason why I mentioned it here :)

Karan Kumar
@knjk04

@/all
Our new organisation: https://github.com/Project-Books ('Book Project' was taken)
Repository: https://github.com/Project-Books/book-project

git clone, git fetch and git pushshould redirect. However, GitHub recommends that we update existing local clones to point to the new URL to avoid confusion using
$ git remote set-url origin https://github.com/Project-Books/book-project

I'll send reminder PRs to update the remote location to all those that are currently assigned to an issue

Karan Kumar
@knjk04
I'm aware the Travis can't find the repository. I'm looking into this.
Also, I'm setting up a Slack workspace, so I'll share the workspace URL soon. In the meantime, feel free to send any messages here
Karan Kumar
@knjk04
It would be good if we could get as many people on here over to Slack. This is now deprecated', so please send future communication over Slack
woodyinho
@woodyinho
Do you have a separate table for custom and predefined shelves?
And also how BookInShelf works?
Karan Kumar
@knjk04
Thanks for your questions. I think you also asked these questions on our Slack workspace (assuming that was you)? If so, I'll just answer the questions there. This Gitter room is now deprecated, so it would be better if all future communication was on Slack. This way, our new joiners can also benefit from the answers