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

Fixed require statements #525

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Fixed require statements #525

wants to merge 7 commits into from

Conversation

Trial97
Copy link

@Trial97 Trial97 commented Mar 22, 2022

The require statements should be dynamic in order to place this project in another folder other than the root config folder

widget/bat.lua Outdated Show resolved Hide resolved
@lcpz
Copy link
Owner

lcpz commented Mar 24, 2022

Thank you for your work. Please see my review.

Before merging, can please you ensure that you tested every widget, layout and utility?

Maybe you can help me with #527 as well.

@Trial97
Copy link
Author

Trial97 commented Mar 25, 2022

Thanks for the review. I reverted the change.
Regarding testing everything it will take me some time as I just started learning lua and awesomewm.
As regards #527 can you elaborate?
It is enough for the files to only have the require statement in them?
e.g.lain-test-quake.lua

local lain      = require('extra.lain')
return lain.util.quake({
    app = 'kitty',
    name = "QuakeDD",
    argname = "--name %s",
    followtag = true,
    settings = function(c)  ...    end
    extra = '',
    border = 1,
    visible = false,
    overlap = false,
    screen = awful.screen.focused(),
    height = 0.25,
    width = 1,
    vert = "top",
    horiz = "left"
})

@lcpz
Copy link
Owner

lcpz commented Mar 25, 2022

As regards #527 can you elaborate?
It is enough for the files to only have the require statement in them?

Unfortunately not, we need to call in each of them all possible functions/widgets with all possible arguments to see if anything breaks in a new commit.

Regarding testing everything it will take me some time as I just started learning lua and awesomewm.

The reason why I mentioned #527 is that if we solve that one, we can also check the validity of this PR.

@lcpz lcpz mentioned this pull request Jun 8, 2022
@lcpz lcpz added this to the release 1.0 milestone Jan 6, 2023
@Trial97
Copy link
Author

Trial97 commented Sep 5, 2023

Ok, a small update to this:

  • added a first set of tests(not all files but maybe)
  • fixed requires to work with both tests and also in an awesome config

Let me know if there are some changes that need to be done regarding tests(if not I will continue adding them in the same manner if I have time)

docs for the future :D

Signed-off-by: Trial97 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants