Convert Bloom to es6 class #428
Conversation
|
The bloom file here is also showing as deleted + new file, can you change that for review? If you did renaming via git maybe you have to do it in two steps like:
? Just an idea, have absolutely no confirmation if this would work better. |
|
(and if so probably accompanied by two separate renaming commits for GitHub to process) |
s1na
force-pushed
the
refactor/bloom-es6
branch
from
263cc8c
to
8fde9d5
Compare
|
Yeah this is a big issue, I have to find a fix. Maybe I'm doing something wrong in my work flow. I re-worked the commit history and even though the first commit clearly shows this is only a rename, but it's still being shown as a removed file and an added file... Should I do it in two PRs? |
|
Can/did you try to do it in two steps like I suggested above and do one commit per step (so first commit just for directory move, second commit for file renaming)? Maybe this becomes easier for GitHub to process? |
|
Found this similar problem description gogs/gogs#5056. Also points in the direction that the suggestion above could eventually fix this. |
|
It is detecting the rename in the first commit, I think it's the large (relative to the file itself) diff that causes it to treat it as a deleted+new file across commits. Created #429 to break it into two PRs. |
Signed-off-by: Sina Mahmoodi <itz.s1na@gmail.com>
Signed-off-by: Sina Mahmoodi <itz.s1na@gmail.com>
Signed-off-by: Sina Mahmoodi <itz.s1na@gmail.com>
|
Ok, have reviewed and merged. |
s1na
force-pushed
the
refactor/bloom-es6
branch
from
8fde9d5
to
c71fcae
Compare
Converts Bloom to an es6 class, and moves it to a directory to have it as a separate module.
My thinking is to structure the VM as multiple modules which contain most of the code, and in the root directory (
lib/) have code that connects these individual modules together. This should also ease our way into a monorepo (if we decided to go for it). Bloom was a low-hanging fruit, I'll create an issue to discuss how to structure the rest of the code.Bloom is used only internally, and is not exposed, so this shouldn't be a breaking change.