-
Notifications
You must be signed in to change notification settings - Fork 156
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
Why rultor use .rultor.yml from base branch and not use it from branch inside pull request #1184
Comments
@yegor256 please, pay attention to this issue |
@Slach rultor uses |
@yegor256 please, pay attention to this issue |
@Slach this code doesn't do anything, but it should actually fetch and merge: https://github.com/yegor256/rultor/blob/1.64.1/src/main/java/com/rultor/profiles/Profiles.java#L96-L99 |
I have a similar behavior for Rultor. I mean the Rultor follows |
@yegor256 there might be security issue here, as someone in pull request may add script: |-
...
env
...
cat your.assets and your variables or assets will be available in logs... Be very careful with such type of feature requests... |
@driver733 I understand the security risks and they should be handled as well. However, from a functional point of view, I update the branch's config with an incompatible change. And the behavior I described should happen only in the branches that have the new changes, but all other branches should behave in the correspondence of their own config - it's like other CI servers work. Now, If I push an incompatible change to master, all other branches will fail until they pull master's changes. And I can't submit a task that updates the Rultor config and does some changes in one pull request. I must split. But this makes review procedure more complex. Again. I understand that there may be a security issue. But this is a non-functional requirement, and it should be handled properly. And this issue is about a feature request. So, security requirements should be a part of the feature implementation as well as other non-functional requirements. @dgroup @driver733 does it make sense? |
@extsoft You're right, it makes sense. I am using Github Actions and it works the following way:
I think this kind of behavior is the most preferable. |
@driver733 @yegor256 now it is by design feature. may be this should be added to documentation? there are many questions about it. And now fail can not be changed at all. #1459 |
see Slach/test_tasks#5 (comment)
i added sudo to my rultor.yml
but when rulton run install
he used old instruction without sudo
why?
The text was updated successfully, but these errors were encountered: