[bookbrainz-site] MonkeyDo merged pull request #589 (master…date-picker-fix): fixes date picker popper position in author section on smaller viewports https://github.com/bookbrainz/bookbrainz-site/pul…
[bookbrainz-site] MonkeyDo reopened pull request #595 (master…feature/collection): Feat[BB-607] : display total number of entity present in a particular collection https://github.com/bookbrainz/bookbrainz-site/pul…
2021-04-06 09622, 2021
_lucifer
alastairp, regarding ascii only keys. i have currently implemented that as key.encode(ENCODING_ASCII, errors='xmlcharrefreplace').decode(ENCODING_ASCII).
2021-04-06 09647, 2021
alastairp
yeah, that looks about right
2021-04-06 09652, 2021
alastairp
are we happy with xmlcharrefreplace
2021-04-06 09613, 2021
akshaaatt[m] uploaded a video: (3757KiB) < https://matrix.org/_matrix/media/r0/download/matrix.org/noavPYfKUjaHfTBxARUjceLS/bandicam%202021-04-06%2017-51-29-170.mp4 >
2021-04-06 09624, 2021
akshaaatt[m]
_lucifer: Have a look at this
2021-04-06 09652, 2021
akshaaatt[m]
It was some extra line of code that was causing the tagger issue XD
2021-04-06 09659, 2021
_lucifer
it was that way hence i kept it. we need some error handling for non ascii characters or do we want to error out?
more a question of which error handler we should use
2021-04-06 09633, 2021
alastairp
xmlcharref or backslash
2021-04-06 09612, 2021
akshaaatt[m]
_lucifer: let me know if this is considered as the fix for the issue or should I work on something more for this? I have pushed the latest fix to the PR 😇
2021-04-06 09640, 2021
_lucifer
alastairp, seems to make more sense to me.
2021-04-06 09649, 2021
_lucifer
*backslash
2021-04-06 09651, 2021
Cyna[m]
bitmap: reosarevok yvanzo I have pushed my proposal for review. I'm currently working on the UI prototype to add images to the proposal till then do have a look into the textual matter and let me know if its not detailed enough
2021-04-06 09603, 2021
_lucifer
backslash seems to make more sense to me.
2021-04-06 09608, 2021
ShivamAwasthi joined the channel
2021-04-06 09629, 2021
alastairp
_lucifer: I wondered if something was missing :)
2021-04-06 09634, 2021
alastairp
sure, sounds good to me
2021-04-06 09611, 2021
_lucifer
akshaaatt[m]: thanks. it looks good but i would want to test it on some older devices as well because i am not sure why it was not implemented this way in the first place.
2021-04-06 09614, 2021
akshaaatt[m]
<_lucifer "akshaaatt: thanks. it looks good"> Great! I am planning to upgrade the dependencies for the project soon. Like the versions and all. Will need to check the compatibility then. I will keep this as a long term goal maybe for the next month as the deadline in case there are issues that need to be resolved. I will even finalize my gsoc proposal in the meantime. If you want any more issues resolved in between,
2021-04-06 09614, 2021
akshaaatt[m]
let me know by mentioning me here 😁
2021-04-06 09630, 2021
ShivamAwasthi has quit
2021-04-06 09645, 2021
_lucifer
alastairp: i have taken a look at your comments on PR 52. a couple of them are already done in other PRs. the remaining I am tracking, I'll open a PR for the leftovers from all the PRs together later.
2021-04-06 09659, 2021
_lucifer
akshaaatt[m]: sure. regarding upgrading the dependencies. we mostly use jetpack which are maintained by google. unless you are changing the major version, you should be good to go.
2021-04-06 09616, 2021
_lucifer
regarding more issues, i don't have any other in mind particularly but i would like to add some more tests. I had cleaned up the code last year, added DI etc to make adding new ones easier.
2021-04-06 09655, 2021
_lucifer
let me know if you are interested in that and we can discuss it in some more detail later.
2021-04-06 09659, 2021
akshaaatt[m]
_lucifer: I am new to writing tests as of now. If you can guide me, I would love that.
2021-04-06 09637, 2021
_lucifer
sure.
2021-04-06 09609, 2021
_lucifer
as you might have already seen we have both unit tests and integration tests (not many though).
2021-04-06 09637, 2021
akshaaatt[m]
<_lucifer "as you might have already seen w"> Right 💯
2021-04-06 09614, 2021
_lucifer
2 particular things i usually keep in mind are: test the API not the implementation and avoid mocks.
this is the CollectionViewModel class, it is used to fetch a user's collections from the MB API
2021-04-06 09633, 2021
akshaaatt[m]
Correct
2021-04-06 09602, 2021
_lucifer
so taking reference from the LookupViewModel class I would suggest you to write a test for this class
2021-04-06 09622, 2021
akshaaatt[m]
Okaaaay! Sounds good.
2021-04-06 09641, 2021
killme joined the channel
2021-04-06 09636, 2021
_lucifer
i see we do not have an API/interface separation in CollectionRepository yet
2021-04-06 09624, 2021
akshaaatt[m]
Yeah we should do that
2021-04-06 09616, 2021
_lucifer
so you can do the following:
2021-04-06 09616, 2021
_lucifer
1) Create a CollectionRepository interface, move the current class to CollectionRepositoryImpl
2021-04-06 09616, 2021
_lucifer
2) Make sure the app uses CollectionRepository interface everywhere
2021-04-06 09616, 2021
_lucifer
3) create a fake class implementing CollectionRepository
2021-04-06 09616, 2021
_lucifer
4) Write a test for the CollectionViewModel by injecting the fake class
2021-04-06 09616, 2021
akshaaatt[m]
_lucifer: Sounds great! Will surely work on all these. This month might get heavy for me because it's the last month of this semester for me. On it though! 😀
2021-04-06 09619, 2021
_lucifer
for sample data to use during testing, create a couple of collections on the website. i would suggest one private and one public at least so that we can test that as well later. store it as a json file where other files are located.
2021-04-06 09637, 2021
_lucifer
for loading the data, we already have a utility function.
2021-04-06 09648, 2021
akshaaatt[m]
<_lucifer "for sample data to use during te"> Yaass 💯
2021-04-06 09618, 2021
_lucifer
sure, no worries. take your time and feel free to ask any queries you might have.
2021-04-06 09648, 2021
_lucifer
(to clarify I refer to using a mocking library as mockito when saying mocks, creating our own fake classes is fine though during unit testing.)
2021-04-06 09612, 2021
akshaaatt[m]
<_lucifer "(to clarify I refer to using a m"> Cool
2021-04-06 09617, 2021
akshaaatt[m]
<_lucifer "sure, no worries. take your time"> Sure!
On Windows / Linux it is ctrl + click, is it command + click on OS?
2021-04-06 09644, 2021
reosarevok
(writing some docs)
2021-04-06 09655, 2021
alastairp
yes, command
2021-04-06 09600, 2021
alastairp
reosarevok:
2021-04-06 09627, 2021
reosarevok
Thanks!
2021-04-06 09658, 2021
BrainzGit
[brainzutils-python] alastair merged pull request #52 (master…namespace-version): BU-25: Cache namespace versions don't work in docker or with distributed hosts https://github.com/metabrainz/brainzutils-python/…
