Rspec local image file processing #38

w00lf · 2019-11-28T08:14:17Z

Added specs for ReverseAsciidoctor: docx and html convert with external URI-s to images

ronaldtse · 2019-11-28T11:13:17Z

@w00lf I'd recommend you implement metanorma/coradoc#84 before this one because it's not easy setting up and use LibreOffice in all platforms...

w00lf · 2019-11-28T12:38:30Z

@w00lf I'd recommend you implement metanorma/coradoc#84 before this one because it's not easy setting up and use LibreOffice in all platforms...

Yes, i have switched to it. Will return to this one after complete it.

ronaldtse · 2019-11-28T13:45:58Z

Sounds good, thanks!

opoudjis · 2019-11-29T01:21:42Z

Fails on Windows:

[!] There was an error parsing `Gemfile`: 
8
[!] There was an error while loading `reverse_adoc.gemspec`: No such file or directory - git ls-files. Bundler cannot continue.
9

10
 #  from D:/a/reverse_adoc/reverse_adoc/reverse_adoc.gemspec:16
11
 #  -------------------------------------------
12
 #    s.add_dependency 'word-to-markdown'
13
 >  end
14
 #  # -*- encoding: utf-8 -*-
15
 #  -------------------------------------------
16
. Bundler cannot continue.
17

18
 #  from D:/a/reverse_adoc/reverse_adoc/Gemfile:6
19
 #  -------------------------------------------
20
 #  
21
 >  gem "byebug"
22
 #  source "https://rubygems.org"

…al URI-s to images

ronaldtse · 2019-12-12T12:30:28Z

Passes on Windows and Linux but fails on Mac?

w00lf · 2019-12-12T12:31:44Z

Yes, for some reason on Mac libreoffice cli tool hangs чт, 12 дек. 2019 г. в 15:30, Ronald Tse <[email protected]>:

Passes on Windows and Linux but fails on Mac? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#38?email_source=notifications&email_token=AAI2XFFW5YUPFUDO6CCVYGDQYIVGLA5CNFSM4JSQ23MKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGWQNKI#issuecomment-564987561>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAI2XFFITPWRSHYKEAOKLCTQYIVGLANCNFSM4JSQ23MA> .

-- С уважением, Третьяков М.

w00lf · 2019-12-13T17:44:10Z

@ronaldtse @opoudjis i have completed with github ci configuration. There is one issue with macos configuration - installed libreoffice cli tool freezes when i try to use it, so i have skipped it for now, left apropriate TODO for it, can you review changes and merge?

w00lf · 2019-12-15T16:02:55Z

Hi there, @CAMOBAP795, can you please review the code i wrote here to see if its ok? I have noticed your comment in here - https://github.com/metanorma/reverse_adoc/blob/master/.github/workflows/macos.yml#L1 is it ok if i change github action yaml?

.github/workflows/macos.yml

.github/workflows/ubuntu.yml

CAMOBAP · 2019-12-15T16:09:02Z

.github/workflows/ubuntu.yml

      - name: Update gems
        run: |
          gem install bundler -v "~> 2"
          bundle install --jobs 4 --retry 3
      - name: Run specs
        run: |
          bundle exec rake
+      - name: Test reverse_adoc binary


The same as above may be better to keep them as ruby tests

Done, moved it

.github/workflows/windows.yml

CAMOBAP

Generally good but some comments need to be addressed

@w00lf potentially, how many repos will need LibreOffice too?

I'm asking because we have many repos with exactly the same CI configuration so we decided to move all configuration to single place https://github.com/metanorma/metanorma-build-scripts/tree/master/ci-master/config/gh-actions and apply it with https://github.com/metanorma/ci-master script

BTW @w00lf I didn't found the line where you call LibreOffice's cli tool, could you please point where it is?

w00lf · 2019-12-15T17:33:21Z

Generally good but some comments need to be addressed

@w00lf potentially, how many repos will need LibreOffice too?

I'm asking because we have many repos with exactly the same CI configuration so we decided to move all configuration to single place https://github.com/metanorma/metanorma-build-scripts/tree/master/ci-master/config/gh-actions and apply it with https://github.com/metanorma/ci-master script

Can tell for sure which repos need Libreoffice which dont, currently this repo need it because it uses this gem: https://github.com/benbalter/word-to-markdown in order to convert docx documents.

BTW @w00lf I didn't found the line where you call LibreOffice's cli tool, could you please point where it is?

Its used inside word-to-markdown gem: https://github.com/benbalter/word-to-markdown/blob/ef1af1ae7750c017d0ed3dd3bfd369a962e858d9/lib/word-to-markdown.rb#L32

ronaldtse · 2019-12-16T06:15:18Z

I’m repeating myself but @camobap795 please do not add libreoffice to metanorma because THIS GEM IS NOT USED BY METANORMA. Thanks. 😉

w00lf · 2019-12-21T11:12:24Z

@CAMOBAP795 can you please review again? Created separate specs for bin shell scripts, removed it from github actions. Also can you tell me how do you use rubocop in your project? I tried to use it from projects folder but it seems it uses completely different rules then the Hound does

ronaldtse · 2019-12-22T13:06:17Z

@opoudjis could you please help review this? Thanks!

w00lf · 2019-12-24T12:55:46Z

@CAMOBAP795 can you please review again?

CAMOBAP · 2019-12-24T14:37:27Z

bin/w2a

@@ -50,9 +51,11 @@ if ReverseAsciidoctor.config.external_images && ReverseAsciidoctor.config.destin
 end

 ReverseAsciidoctor.config.sourcedir = Dir.mktmpdir
+# TODO: Check if needed


Placing TODO is good practice, alongside with this I propose to report an issue also

Because sometimes those todo being ignored

CAMOBAP · 2019-12-24T14:37:37Z

bin/w2a

 # puts "ReverseAsciidoctor.config.sourcedir #{ReverseAsciidoctor.config.sourcedir}"

 doc = WordToMarkdown.new(filename, ReverseAsciidoctor.config.sourcedir)
+# TODO: Check if needed


The same comment about TODO

CAMOBAP · 2019-12-24T14:37:44Z

spec/bin/w2a_spec.rb

+  subject(:convert) do
+    ShellUtils.execute!("./bin/w2a -e -o test1 #{input_file_path}")
+  end
+  # TODO: fix github actions integration with libreoffice, currently it hangs


The same comment about TODO

CAMOBAP · 2019-12-24T14:37:50Z

spec/lib/reverse_asciidoctor_spec.rb

+    end
+  end
+
+  # TODO: fix github actions integration with libreoffice, currently it hangs


The same comment about TODO

CAMOBAP

Looks good for me

P.S. my comment about TODO comments up to you, it should not stop you from merging this PR

ronaldtse · 2019-12-25T10:38:23Z

Two approvals and merging. @w00lf could you help create those issues? Thanks!

w00lf · 2019-12-25T12:21:59Z

Two approvals and merging. @w00lf could you help create those issues? Thanks!

Will do

w00lf force-pushed the feature/rspec-local-image-file-processing branch from 505df1c to 31a3df3 Compare November 28, 2019 09:52

w00lf added 4 commits December 12, 2019 12:45

Added specs for ReverseAsciidoctor: docx and html convert with extern…

575e98d

…al URI-s to images

Fix macos run, changed windows path set

9991c15

Hound fix

634c4bb

Macos fix --headless flag

f0ba495

w00lf force-pushed the feature/rspec-local-image-file-processing branch from 181683d to f0ba495 Compare December 12, 2019 09:59

w00lf added 6 commits December 13, 2019 16:37

Macos libreoffice workaround

b104b6e

Try linked binary - soffice

100c0b7

another brew version of libreoffice

0ee47a3

Skipped completely docx conversion test suite on macos

c4c24ce

Test binary - macos, linux, fix reverse_adoc binary

46b923e

Hud fix, macos workflow

09e4889

CAMOBAP reviewed Dec 15, 2019

View reviewed changes

.github/workflows/macos.yml Outdated Show resolved Hide resolved

CAMOBAP reviewed Dec 15, 2019

View reviewed changes

.github/workflows/ubuntu.yml Show resolved Hide resolved

CAMOBAP reviewed Dec 15, 2019

View reviewed changes

.github/workflows/windows.yml Show resolved Hide resolved

CAMOBAP requested changes Dec 15, 2019

View reviewed changes

CAMOBAP mentioned this pull request Dec 15, 2019

Add LibreOffice dependency metanorma/metanorma-snap#3

Closed

w00lf added 2 commits December 21, 2019 13:41

Created specs for bin shell files

83d93bd

Code review, skip windows bin specs

c7f8b6e

w00lf added 3 commits December 21, 2019 14:01

Hound fixes

9f5f46b

Houd fixes

c37e735

Houd fix

cc5887f

ronaldtse requested a review from opoudjis December 22, 2019 13:06

ronaldtse assigned opoudjis Dec 22, 2019

opoudjis approved these changes Dec 24, 2019

View reviewed changes

CAMOBAP reviewed Dec 24, 2019

View reviewed changes

CAMOBAP approved these changes Dec 24, 2019

View reviewed changes

ronaldtse merged commit d30dbcc into master Dec 25, 2019

This was referenced Jan 18, 2020

rspec local image file processing #32

Closed

Test reverse_adoc with data URI using external images #28

Closed

ronaldtse mentioned this pull request May 23, 2024

HTML document work metanorma/coradoc#61

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rspec local image file processing #38

Rspec local image file processing #38

w00lf commented Nov 28, 2019

ronaldtse commented Nov 28, 2019

w00lf commented Nov 28, 2019

ronaldtse commented Nov 28, 2019

opoudjis commented Nov 29, 2019

ronaldtse commented Dec 12, 2019

w00lf commented Dec 12, 2019 via email

w00lf commented Dec 13, 2019

w00lf commented Dec 15, 2019

CAMOBAP Dec 15, 2019

w00lf Dec 21, 2019

CAMOBAP left a comment •

edited

Loading

w00lf commented Dec 15, 2019

ronaldtse commented Dec 16, 2019

w00lf commented Dec 21, 2019

ronaldtse commented Dec 22, 2019

w00lf commented Dec 24, 2019

CAMOBAP Dec 24, 2019

w00lf Dec 25, 2019

CAMOBAP Dec 24, 2019

w00lf Dec 25, 2019

CAMOBAP Dec 24, 2019

w00lf Dec 25, 2019

CAMOBAP Dec 24, 2019

w00lf Dec 25, 2019

CAMOBAP left a comment •

edited

Loading

ronaldtse commented Dec 25, 2019

w00lf commented Dec 25, 2019

Rspec local image file processing #38

Rspec local image file processing #38

Conversation

w00lf commented Nov 28, 2019

ronaldtse commented Nov 28, 2019

w00lf commented Nov 28, 2019

ronaldtse commented Nov 28, 2019

opoudjis commented Nov 29, 2019

ronaldtse commented Dec 12, 2019

w00lf commented Dec 12, 2019 via email

w00lf commented Dec 13, 2019

w00lf commented Dec 15, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CAMOBAP left a comment • edited Loading

Choose a reason for hiding this comment

w00lf commented Dec 15, 2019

ronaldtse commented Dec 16, 2019

w00lf commented Dec 21, 2019

ronaldtse commented Dec 22, 2019

w00lf commented Dec 24, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CAMOBAP left a comment • edited Loading

Choose a reason for hiding this comment

ronaldtse commented Dec 25, 2019

w00lf commented Dec 25, 2019

CAMOBAP left a comment •

edited

Loading

CAMOBAP left a comment •

edited

Loading